| *** r-daneel has quit IRC | 00:01 | |
| *** naohirot has joined #openstack-ironic | 00:09 | |
| *** Nisha has quit IRC | 00:16 | |
| *** achanda has quit IRC | 00:18 | |
| *** achanda has joined #openstack-ironic | 00:18 | |
| *** kozhukalov has quit IRC | 00:20 | |
| *** kkoski has quit IRC | 00:20 | |
| *** mtanino has quit IRC | 00:21 | |
| *** achanda has quit IRC | 00:23 | |
| *** david-lyle_afk has quit IRC | 00:34 | |
| *** Marga_ has quit IRC | 00:40 | |
| *** yuanying has joined #openstack-ironic | 00:51 | |
| *** romcheg has quit IRC | 00:57 | |
| *** yuanying has quit IRC | 01:02 | |
| *** ChuckC has quit IRC | 01:10 | |
| *** chenleji has joined #openstack-ironic | 01:20 | |
| *** david-lyle_afk has joined #openstack-ironic | 01:21 | |
| *** devlaps has joined #openstack-ironic | 01:31 | |
| *** chenglch has joined #openstack-ironic | 01:32 | |
| *** stendulker has joined #openstack-ironic | 01:40 | |
| *** ChuckC has joined #openstack-ironic | 01:42 | |
| *** Nisha has joined #openstack-ironic | 01:44 | |
| *** Nisha_away has joined #openstack-ironic | 01:45 | |
| *** Nisha has quit IRC | 01:45 | |
| *** ChuckC has quit IRC | 01:46 | |
| *** ChuckC has joined #openstack-ironic | 01:47 | |
| *** devlaps has quit IRC | 01:51 | |
| *** rwsu is now known as rwsu-afk | 01:52 | |
| lintan | Hi NobodyCam, | 01:58 |
|---|---|---|
| *** Nisha_away has quit IRC | 02:01 | |
| *** spandhe has quit IRC | 02:06 | |
| *** Haomeng|2 has quit IRC | 02:06 | |
| *** harlowja_ is now known as harlowja_away | 02:09 | |
| *** coolsvap has joined #openstack-ironic | 02:29 | |
| *** david-lyle_afk has quit IRC | 02:36 | |
| *** yog_ has joined #openstack-ironic | 02:39 | |
| *** anderbubble has joined #openstack-ironic | 02:43 | |
| *** killer_prince is now known as lazy_prince | 02:43 | |
| *** stendulker has quit IRC | 02:50 | |
| *** lintan has quit IRC | 02:52 | |
| *** achanda has joined #openstack-ironic | 02:58 | |
| *** achanda has quit IRC | 03:10 | |
| *** coolsvap is now known as coolsvap|afk | 03:15 | |
| *** ijw has quit IRC | 03:17 | |
| *** david-lyle_afk has joined #openstack-ironic | 03:17 | |
| *** Nisha has joined #openstack-ironic | 03:18 | |
| *** lintan has joined #openstack-ironic | 03:20 | |
| *** coolsvap|afk is now known as coolsvap | 03:22 | |
| openstackgerrit | Nisha Agarwal proposed openstack/python-ironicclient: Add support for inspection to node-set-provision-state https://review.openstack.org/148804 | 03:35 |
| *** lazy_prince is now known as killer_prince | 03:36 | |
| *** jgrimm is now known as zz_jgrimm | 03:40 | |
| *** yuanying has joined #openstack-ironic | 03:41 | |
| *** zz_jgrimm is now known as jgrimm | 03:47 | |
| *** chenleji has quit IRC | 03:53 | |
| *** jmccrory has joined #openstack-ironic | 03:59 | |
| openstackgerrit | SHIGEMATSU Mitsuhiro proposed openstack/ironic-specs: Fix typo in ironic-specs/specs/kilo/uefi-secure-boot.rst https://review.openstack.org/162590 | 04:00 |
| *** Marga_ has joined #openstack-ironic | 04:04 | |
| *** Marga_ has quit IRC | 04:04 | |
| *** Marga_ has joined #openstack-ironic | 04:05 | |
| *** achanda has joined #openstack-ironic | 04:07 | |
| *** spandhe has joined #openstack-ironic | 04:08 | |
| *** spandhe_ has joined #openstack-ironic | 04:12 | |
| *** spandhe has quit IRC | 04:12 | |
| *** spandhe_ is now known as spandhe | 04:12 | |
| *** stendulker has joined #openstack-ironic | 04:19 | |
| *** anderbubble has quit IRC | 04:29 | |
| *** absubram has joined #openstack-ironic | 04:35 | |
| openstackgerrit | Yuiko Takada proposed stackforge/ironic-discoverd: [WIP]Add supporting generate config files https://review.openstack.org/163281 | 04:38 |
| *** rameshg87 has joined #openstack-ironic | 04:46 | |
| openstackgerrit | Michael Davies proposed openstack/ironic-specs: API Microversions https://review.openstack.org/161110 | 04:51 |
| openstackgerrit | Michael Davies proposed openstack/ironic-specs: API Microversions https://review.openstack.org/161110 | 04:52 |
| openstackgerrit | SHIGEMATSU Mitsuhiro proposed openstack/ironic-specs: Fix typos in ironic-specs/specs/kilo/drac-bios-mgmt.rst https://review.openstack.org/162578 | 04:55 |
| rameshg87 | good morning ironic | 04:58 |
| openstackgerrit | Pradhan proposed stackforge/proliantutils: Update RIS library https://review.openstack.org/163290 | 05:02 |
| *** kozhukalov has joined #openstack-ironic | 05:06 | |
| openstackgerrit | Yuiko Takada proposed stackforge/ironic-discoverd: [WIP]Add supporting generate config files https://review.openstack.org/163281 | 05:11 |
| mrda | morning rameshg87 | 05:17 |
| rameshg87 | good afternoon mrda | 05:19 |
| mrda | ;) | 05:19 |
| openstackgerrit | Nisha Agarwal proposed openstack/ironic: iLO implementation for hardware inspection https://review.openstack.org/151596 | 05:27 |
| *** killer_prince is now known as lazy_prince | 05:28 | |
| *** Haomeng has joined #openstack-ironic | 05:28 | |
| *** achanda has quit IRC | 05:31 | |
| openstackgerrit | Nisha Agarwal proposed openstack/ironic: iLO implementation for hardware inspection https://review.openstack.org/151596 | 05:33 |
| *** achanda has joined #openstack-ironic | 05:34 | |
| *** lazy_prince has quit IRC | 05:39 | |
| *** killer_prince has joined #openstack-ironic | 05:42 | |
| *** killer_prince is now known as lazy_prince | 05:42 | |
| *** achanda has quit IRC | 05:48 | |
| *** bnemec has quit IRC | 05:49 | |
| *** bnemec has joined #openstack-ironic | 05:52 | |
| *** achanda has joined #openstack-ironic | 05:52 | |
| *** achanda has quit IRC | 05:58 | |
| *** achanda has joined #openstack-ironic | 06:01 | |
| *** pradipta has joined #openstack-ironic | 06:09 | |
| *** achanda has quit IRC | 06:14 | |
| *** dmellado has quit IRC | 06:15 | |
| *** stendulker_ has joined #openstack-ironic | 06:15 | |
| *** stendulker has quit IRC | 06:17 | |
| *** dmellado has joined #openstack-ironic | 06:19 | |
| *** jmccrory has quit IRC | 06:20 | |
| *** andreykurilin_ has joined #openstack-ironic | 06:28 | |
| openstackgerrit | Yuiko Takada proposed stackforge/ironic-discoverd: [WIP]Add supporting generate config files https://review.openstack.org/163281 | 06:42 |
| *** andreykurilin_ has quit IRC | 06:57 | |
| *** spandhe has quit IRC | 06:58 | |
| *** kevinbenton has quit IRC | 06:59 | |
| *** dlpartain has joined #openstack-ironic | 07:16 | |
| *** dlpartain has left #openstack-ironic | 07:20 | |
| *** lazy_prince is now known as killer_prince | 07:26 | |
| *** pas-ha has joined #openstack-ironic | 07:38 | |
| *** ukalifon1 has joined #openstack-ironic | 07:39 | |
| GheRivero | morning | 07:52 |
| *** coolsvap is now known as coolsvap|brb | 07:54 | |
| *** chlong has quit IRC | 08:01 | |
| *** stendulker has joined #openstack-ironic | 08:02 | |
| *** romcheg has joined #openstack-ironic | 08:03 | |
| *** jcoufal has joined #openstack-ironic | 08:04 | |
| *** stendulker_ has quit IRC | 08:04 | |
| *** pradipta has quit IRC | 08:16 | |
| *** ifarkas has joined #openstack-ironic | 08:25 | |
| *** coolsvap|brb is now known as coolsvap | 08:26 | |
| *** killer_prince is now known as lazy_prince | 08:33 | |
| *** takadayuiko has joined #openstack-ironic | 08:35 | |
| *** ndipanov has joined #openstack-ironic | 08:47 | |
| *** jiangfei has quit IRC | 08:52 | |
| *** jiangfei has joined #openstack-ironic | 08:54 | |
| Nisha | GheRivero, morning | 08:56 |
| openstackgerrit | Aparna proposed stackforge/proliantutils: Proliantutils support for configuring httpboot through RIS https://review.openstack.org/163325 | 08:56 |
| *** marios has joined #openstack-ironic | 08:57 | |
| *** athomas has quit IRC | 09:06 | |
| *** athomas has joined #openstack-ironic | 09:12 | |
| *** athomas has joined #openstack-ironic | 09:13 | |
| *** tiagogomes_ has joined #openstack-ironic | 09:18 | |
| openstackgerrit | Ramakrishnan G proposed openstack/ironic: Add label to virtual floppy image https://review.openstack.org/162387 | 09:21 |
| rameshg87 | naohirot, ^^ | 09:22 |
| naohirot | hi rameshg87 :) | 09:23 |
| *** stendulker has quit IRC | 09:23 | |
| rameshg87 | naohirot, hello, just changed the label | 09:23 |
| naohirot | rameshg87: thank you for considering it :) | 09:24 |
| *** pradipta has joined #openstack-ironic | 09:24 | |
| rameshg87 | naohirot, :) | 09:24 |
| rameshg87 | naohirot, the dib ramdisk might require some more changes | 09:24 |
| openstackgerrit | Ramakrishnan G proposed openstack/ironic-python-agent: Use labels for virtual media dev in agent ramdisk https://review.openstack.org/162392 | 09:24 |
| naohirot | rameshg87: I think it's really great idea to use /dev/disk/label | 09:24 |
| rameshg87 | naohirot, i was trying to make changes for it, but it doesn't have /dev/disk directory | 09:25 |
| rameshg87 | naohirot, probably the linux kernel there is not configured to map devices by label | 09:25 |
| *** jistr has joined #openstack-ironic | 09:25 | |
| rameshg87 | naohirot, i think it requires some udev rules change | 09:25 |
| rameshg87 | naohirot, i will try to see that today | 09:25 |
| naohirot | rameshg87: s/"dev/disk/label"/"/dev/disk/by-lable/"/ | 09:26 |
| naohirot | rameshg87: I see | 09:27 |
| *** kevinbenton has joined #openstack-ironic | 09:27 | |
| naohirot | rameshg87: are we going to change DIB to this way if you confirmed it worked okay? | 09:28 |
| rameshg87 | naohirot, yeah .. i made the changes to dib on similar lines to ipa | 09:28 |
| rameshg87 | naohirot, but dib doesn't have /dev/disk-by-label directory | 09:28 |
| rameshg87 | naohirot, i need to check what udev rules need to be added for that | 09:28 |
| rameshg87 | or is it udev related | 09:29 |
| naohirot | rameshg87: I see, is it due to some difference between syslinux and coreos? | 09:29 |
| *** stendulker has joined #openstack-ironic | 09:30 | |
| rameshg87 | naohirot, i suspect this: https://github.com/openstack/diskimage-builder/blob/master/elements/ramdisk/init.d/10-start-base-system#L7-L45 | 09:30 |
| rameshg87 | naohirot, somewhere we need to tell linux kernel to create /dev/disk/by-label directory and devices under it | 09:31 |
| naohirot | rameshg87: I see, it's not so simple :) | 09:31 |
| rameshg87 | naohirot, but since it's already there, i hope it will be simple :) | 09:32 |
| rameshg87 | naohirot, will check and keep you posted | 09:32 |
| naohirot | rameshg87: okay, great | 09:32 |
| *** lucasagomes has joined #openstack-ironic | 09:34 | |
| naohirot | rameshg87: maybe I should read some bp and code first and then ask | 09:35 |
| naohirot | rameshg87: but can I ask some question regarding https://blueprints.launchpad.net/ironic/+spec/local-boot-support-with-partition-images | 09:35 |
| naohirot | rameshg87: and https://review.openstack.org/#/c/156608/ if you have time | 09:36 |
| rameshg87 | naohirot, yeah | 09:36 |
| *** stendulker has quit IRC | 09:36 | |
| naohirot | rameshg87: does this patch make iscsi_ilo boot user os without boot ramdisk? | 09:37 |
| naohirot | rameshg87: like IPA? | 09:38 |
| rameshg87 | naohirot, yeah it does | 09:38 |
| *** stendulker has joined #openstack-ironic | 09:38 | |
| naohirot | rameshg87: that's interesting, basically how? | 09:38 |
| rameshg87 | naohirot, after this change the bootloader from the deployed image will be installed | 09:39 |
| naohirot | rameshg87: do we write MBR in case of pxe deploy too? | 09:39 |
| rameshg87 | naohirot, yeah | 09:39 |
| naohirot | rameshg87: do we need whole user os image like IPA? | 09:40 |
| naohirot | rameshg87: what I'd like to know is how to create user os image? is it same as IPA? | 09:41 |
| rameshg87 | naohirot, yeah, the user image needs to have grub in it | 09:42 |
| rameshg87 | naohirot, then the ramdisk will go and run /usr/sbin/grub-install on the bare metal after chrooting the to the deploy os image | 09:43 |
| rameshg87 | naohirot, https://github.com/openstack/diskimage-builder/blob/master/elements/deploy-ironic/init.d/80-deploy-ironic#L64-L111 | 09:43 |
| *** erwan_taf has joined #openstack-ironic | 09:44 | |
| naohirot | rameshg87: In order to understand how it works, I'd like to port iscsi_ilo code to irmc and try to run it. | 09:45 |
| *** erwan_taf has left #openstack-ironic | 09:45 | |
| rameshg87 | naohirot, yeah sure .. | 09:45 |
| rameshg87 | naohirot, iscsi_ilo is expected to work | 09:45 |
| naohirot | rameshg87: Okay, but I don't have hp machine :) | 09:46 |
| naohirot | rameshg87: Is the command line same as IPA "disk-image-create ubuntu baremetal vm dhcp-all-interfaces -o my-image"? | 09:47 |
| naohirot | rameshg87: in order to invoke 80-deploy-ironic#L64-L111, ironic element require for user image? | 09:48 |
| naohirot | rameshg87: like this ? "disk-image-create ubuntu baremetal vm ironic dhcp-all-interfaces" | 09:49 |
| *** jistr has quit IRC | 09:50 | |
| rameshg87 | naohirot, yeah | 09:52 |
| *** jistr has joined #openstack-ironic | 09:52 | |
| rameshg87 | naohirot, but it doesn't create images with grub right now | 09:52 |
| rameshg87 | naohirot, it is *assumed* for now that grub is installed on the os-image | 09:52 |
| openstackgerrit | Ramakrishnan G proposed openstack/ironic: Add localboot support for uefi boot mode https://review.openstack.org/159855 | 09:53 |
| naohirot | rameshg87: thanks, I got enough hints to read code and doc so far, I'll ask you again after reading those. | 09:55 |
| rameshg87 | naohirot, sure .. | 09:55 |
| naohirot | rameshg87: I got to go, see you later! | 09:56 |
| rameshg87 | naohirot, bye, have a nice day !! | 09:56 |
| naohirot | rameshg87: bye :) | 09:56 |
| *** naohirot has quit IRC | 09:58 | |
| lucasagomes | rameshg87, hi there, for the uefi local boot | 10:05 |
| rameshg87 | lucasagomes, hi | 10:06 |
| lucasagomes | could we add a label on the EFI paritition? and on DIB we just search for that label instead of picking the first partition on the disk? | 10:06 |
| lucasagomes | sounds a bit more reliable no? | 10:06 |
| rameshg87 | lucasagomes, yeah it does | 10:06 |
| lucasagomes | I know on DIB I serach for the last one for the root parition | 10:06 |
| lucasagomes | but that's because it's the user image, so I don't create the filesystem there | 10:06 |
| lucasagomes | (on IPA I pass the filesystem UUID so it's more reliable than DIB) | 10:07 |
| rameshg87 | lucasagomes, yeah labels is a much better one . | 10:07 |
| rameshg87 | lucasagomes, but dib ramdisk is not having /dev/disk-by-label/ for some reason | 10:07 |
| rameshg87 | lucasagomes, i think i need to have some udev rules for that | 10:07 |
| rameshg87 | lucasagomes, for telling linux kernel to create those soft links | 10:07 |
| lucasagomes | rameshg87, oh, right... yeah you would have to do a lsblk or something | 10:07 |
| lucasagomes | to find out the label | 10:08 |
| lucasagomes | it's fine, can be a later improvement anyway | 10:08 |
| lucasagomes | rameshg87, localboot is merged on IPA as well, in case u want to add support there too | 10:08 |
| rameshg87 | lucasagomes, yeah i am making changes in ipa :) | 10:08 |
| rameshg87 | lucasagomes, will have a patch by today | 10:08 |
| lucasagomes | rameshg87, awesome :) | 10:09 |
| lucasagomes | please pass the filesystem UUID via the call instead of picking the first partition there :D | 10:09 |
| rameshg87 | lucasagomes, label ? | 10:09 |
| rameshg87 | lucasagomes, did you mean label ? | 10:10 |
| rameshg87 | lucasagomes, using constant labels is a bit problematic | 10:10 |
| rameshg87 | lucasagomes, if we installed on one disk using root device hints | 10:10 |
| lucasagomes | rameshg87, no I mean UUID | 10:10 |
| rameshg87 | lucasagomes, so figure out uuid from ironic itself and then pass ? | 10:11 |
| lucasagomes | sudo blkid -s UUID -o value /dev/sda1 | 10:11 |
| lucasagomes | 68ccb940-ff05-44c7-8d0c-35d1b0f57252 | 10:11 |
| lucasagomes | for e.g | 10:11 |
| rameshg87 | lucasagomes, does fat32 partitions have UUID ? :) | 10:11 |
| rameshg87 | checking ... | 10:11 |
| lucasagomes | rameshg87, I thought it have | 10:11 |
| * lucasagomes thinks now | 10:11 | |
| lucasagomes | maybe not :/ I don't have a vfat part here to test | 10:12 |
| lucasagomes | rameshg87, right, but we can search for the label on that specific disk and not across all disks right? | 10:13 |
| rameshg87 | dd if=/dev/zero of=image.img bs=1M count=100 && mkfs.vfat image.img | 10:13 |
| rameshg87 | blkid -s UUID -o value image.img | 10:13 |
| rameshg87 | 634A-49C3 | 10:13 |
| rameshg87 | gives me something | 10:13 |
| rameshg87 | :) | 10:13 |
| lucasagomes | :) | 10:13 |
| lucasagomes | fair enuff | 10:13 |
| rameshg87 | lucasagomes, yeah it changes .. when i do mkfs again | 10:14 |
| rameshg87 | lucasagomes, so reliable :) | 10:14 |
| lucasagomes | yeah I kinda dislike relying on partition orders | 10:14 |
| lucasagomes | so it's always better to use a unique identifier for it | 10:14 |
| lucasagomes | rameshg87, thanks! | 10:14 |
| rameshg87 | agreed | 10:14 |
| rameshg87 | lucasagomes, i think better to make that change | 10:14 |
| rameshg87 | lucasagomes, do you want to leave a comment ? | 10:14 |
| lucasagomes | I think I did on both patches already | 10:15 |
| lucasagomes | tho I said label, we can say UUID :) | 10:15 |
| * rameshg87 looks | 10:16 | |
| rameshg87 | lucasagomes, i think it's there only in chat | 10:16 |
| rameshg87 | lucasagomes, oh got it | 10:17 |
| * rameshg87 forgot to refresh | 10:17 | |
| rameshg87 | :D | 10:17 |
| lucasagomes | heh | 10:17 |
| *** romcheg has quit IRC | 10:20 | |
| *** chlong has joined #openstack-ironic | 10:23 | |
| *** stendulker_ has joined #openstack-ironic | 10:30 | |
| *** romcheg has joined #openstack-ironic | 10:31 | |
| *** stendulker has quit IRC | 10:33 | |
| openstackgerrit | Nisha Agarwal proposed openstack/ironic: Automate boot iso creation with in ironic for iscsi-ilo https://review.openstack.org/155900 | 10:33 |
| *** jistr has quit IRC | 10:38 | |
| *** jistr has joined #openstack-ironic | 10:39 | |
| *** Nisha_away has joined #openstack-ironic | 10:39 | |
| stendulker_ | Hi lucasgomes: this is regarding code review for defect fix https://review.openstack.org/#/c/155731 - Ilo drivers sets capabilities:boot_mode in node | 10:40 |
| *** Kinnison has left #openstack-ironic | 10:40 | |
| *** Nisha has quit IRC | 10:41 | |
| BadCub | morning Ironic | 10:42 |
| stendulker_ | lucasgomes: Had replied to your queries. Did you get chance to go through that. | 10:42 |
| *** yuanying has quit IRC | 10:43 | |
| *** Nisha_away has quit IRC | 10:43 | |
| *** chenglch has quit IRC | 10:44 | |
| *** kevinbenton has quit IRC | 10:45 | |
| *** Marga_ has quit IRC | 10:55 | |
| openstackgerrit | Lucas Alvares Gomes proposed openstack/ironic: PXE driver: Deprecate pxe_deploy_{ramdisk, kernel} https://review.openstack.org/158644 | 10:56 |
| lucasagomes | stendulker_, will take a look | 10:56 |
| lucasagomes | BadCub, good morning | 10:56 |
| BadCub | morning lucasagomes | 10:56 |
| stendulker_ | lucasgomes, thank you | 10:57 |
| *** Marga_ has joined #openstack-ironic | 10:57 | |
| openstackgerrit | Ramakrishnan G proposed openstack/ironic: Add localboot support for uefi boot mode https://review.openstack.org/159855 | 11:04 |
| *** takadayuiko has quit IRC | 11:06 | |
| *** tiagogomes_ has quit IRC | 11:19 | |
| *** Marga_ has quit IRC | 11:20 | |
| *** tiagogomes_ has joined #openstack-ironic | 11:23 | |
| *** romcheg1 has joined #openstack-ironic | 11:35 | |
| *** romcheg has quit IRC | 11:35 | |
| BadCub | hey lucasagomes were you able to get t=gate tests done on https://blueprints.launchpad.net/ironic/+spec/ipa-as-default-ramdisk ? | 11:35 |
| *** stendulker_ has quit IRC | 11:35 | |
| BadCub | brb | 11:38 |
| *** pelix has joined #openstack-ironic | 11:39 | |
| *** rameshg87 has quit IRC | 11:39 | |
| openstackgerrit | John L. Villalovos proposed openstack/ironic: Update unittests for _make_password_file https://review.openstack.org/162672 | 11:42 |
| openstackgerrit | Merged openstack/ironic: Update API doc to reflect node uuid or name https://review.openstack.org/163009 | 11:42 |
| openstackgerrit | Merged openstack/ironic: Add label to virtual floppy image https://review.openstack.org/162387 | 11:43 |
| *** vdrok_afk is now known as vdrok | 11:44 | |
| *** lazy_prince is now known as killer_prince | 11:46 | |
| lucasagomes | BadCub, sorry I was on a call | 11:47 |
| lucasagomes | yes, it's now running a check on every patch | 11:47 |
| lucasagomes | non-voting for now, but that's fine because we need to gather some data before making it voting | 11:48 |
| lucasagomes | BadCub, I have also a new patch adding it to the ironic-python-agent gate check | 11:48 |
| lucasagomes | https://review.openstack.org/#/c/163344/ | 11:48 |
| lucasagomes | BadCub, but in Ironic it's all set :) | 11:48 |
| BadCub | Nice! | 11:49 |
| BadCub | Updated it on the spreadsheet as completed | 11:51 |
| lucasagomes | BadCub, awesome! thanks much | 11:52 |
| BadCub | YW bud :-) | 11:52 |
| lucasagomes | (: | 11:52 |
| *** killer_prince is now known as lazy_prince | 12:08 | |
| *** pradipta has quit IRC | 12:11 | |
| Shrews | BadCub: wth are you doing up so freaking early????? | 12:12 |
| Shrews | i mean... good morning :-P | 12:12 |
| BadCub | insomnia Shrews. So figured I would get some work done LOL | 12:13 |
| BadCub | Morning to you too! :-) | 12:13 |
| *** dprince has joined #openstack-ironic | 12:20 | |
| openstackgerrit | Lucas Alvares Gomes proposed openstack/ironic: PXE driver: Deprecate pxe_deploy_{ramdisk, kernel} https://review.openstack.org/158644 | 12:26 |
| *** lucasagomes is now known as lucas-hungry | 12:26 | |
| NobodyCam | Good Morning Ironic | 12:52 |
| *** rloo has joined #openstack-ironic | 12:55 | |
| Shrews | Hey NobodyCam | 12:55 |
| NobodyCam | morning Shrews :) | 12:55 |
| jroll | morning all :) | 12:56 |
| jroll | https://code.facebook.com/posts/1601610310055392/introducing-openbmc-an-open-software-framework-for-next-generation-system-management/ | 12:56 |
| NobodyCam | morning jroll | 12:56 |
| Shrews | morning jroll | 12:56 |
| jroll | heya Shrews NobodyCam BadCub lucas-hungry and openstackgerrit! | 12:57 |
| Shrews | jroll: you don't have any scripts to create VMs that you might use with agent_ssh, do you? | 12:57 |
| jroll | Shrews: cat devstack/lib/ironic | 12:58 |
| Shrews | jroll: yeah, that's what i'm currently going by | 12:58 |
| jroll | that's about all I've got, sorry | 12:58 |
| Shrews | jroll: USELESS!! YOU'RE FIRED! | 12:58 |
| jroll | what hypervisor are you using? | 12:58 |
| jroll | oh ok | 12:58 |
| * jroll goes back to bed | 12:58 | |
| NobodyCam | night jroll :-p | 12:59 |
| Shrews | same as devstack | 12:59 |
| jroll | :P | 12:59 |
| NobodyCam | and that openbmc is insterstering | 12:59 |
| jroll | Shrews: yeah, I tried to do this once without devstack and it ended in tears | 12:59 |
| jroll | NobodyCam: also https://github.com/Broadcom-Switch/OpenNSL and https://code.facebook.com/posts/843620439027582/facebook-open-switching-system-fboss-and-wedge-in-the-open/ | 13:01 |
| NobodyCam | Shrews: could you use the tripleo create-nodes script? | 13:02 |
| Shrews | NobodyCam: maybe? where can i find it? | 13:04 |
| jroll | os-tripleo-create-nodes-config | 13:05 |
| NobodyCam | Shrews: https://github.com/openstack/tripleo-incubator/blob/master/scripts/create-nodes | 13:06 |
| Shrews | LOL. Looks a lot like what I already have (and what devstack has) | 13:07 |
| Shrews | will look closer though. thx | 13:08 |
| NobodyCam | :-p | 13:08 |
| *** kkoski has joined #openstack-ironic | 13:08 | |
| *** jjohnson2 has joined #openstack-ironic | 13:10 | |
| openstackgerrit | Merged openstack/ironic: Updated from global requirements https://review.openstack.org/161811 | 13:12 |
| openstackgerrit | Zhenguo Niu proposed openstack/ironic: Fix some typos https://review.openstack.org/163422 | 13:19 |
| victor_lowther | Good morning Ironic | 13:22 |
| jjohnson2 | good morning | 13:23 |
| jjohnson2 | of those who have seen my console manager (or want to see said video), did anyone have comment? | 13:23 |
| jroll | jjohnson2: I don't have any comments on that, but you might be interested in https://code.facebook.com/posts/1601610310055392/introducing-openbmc-an-open-software-framework-for-next-generation-system-management/ | 13:25 |
| jroll | :) | 13:25 |
| jroll | \o victor_lowther | 13:25 |
| victor_lowther | jroll: I think openbmc is a great idea | 13:27 |
| jroll | +1 | 13:27 |
| jroll | I'm excited | 13:27 |
| victor_lowther | I would live to be able to fiincredibly lame bmc firmware glitches. | 13:27 |
| victor_lowther | er, fix | 13:28 |
| jroll | mhmmm. | 13:28 |
| jroll | also make them secure. | 13:28 |
| NobodyCam | morning jjohnson2 & victor_lowther | 13:29 |
| jjohnson2 | NobodyCam, morning | 13:29 |
| *** lucas-hungry is now known as lucasagomes | 13:29 | |
| lucasagomes | jroll, Shrews victor_lowther NobodyCam jjohnson2 morning :) | 13:29 |
| jjohnson2 | lucasagomes, good morning | 13:29 |
| jroll | morning lucasagomes :) | 13:30 |
| NobodyCam | morning lucasagomes | 13:30 |
| Shrews | hi lucasagomes | 13:30 |
| NobodyCam | oh jjohnson2 have you seen: http://blog.nemebean.com/content/ipmi-controller-openstack-instances | 13:30 |
| jjohnson2 | jroll, interesting indeed | 13:30 |
| lucasagomes | jroll, btw, https://review.openstack.org/#/c/163344/ I think it makes sense to add there too | 13:30 |
| jroll | lucasagomes: agree, though we should make it build from src no? | 13:31 |
| *** jcoufal has quit IRC | 13:31 | |
| jroll | lucasagomes: otherwise it's useless :) | 13:31 |
| lucasagomes | jroll, *facepalm* | 13:31 |
| lucasagomes | +1111111 | 13:31 |
| lucasagomes | indeed, forgot that bit :) makes total sense | 13:32 |
| *** jcoufal has joined #openstack-ironic | 13:32 | |
| jroll | heh | 13:32 |
| jroll | left a -1 for you | 13:32 |
| lucasagomes | thanks :) | 13:32 |
| jjohnson2 | NobodyCam, interesting... it's similar to what Martini was doing | 13:32 |
| NobodyCam | jjohnson2: Using the impi listener you've done :) | 13:33 |
| jjohnson2 | NobodyCam, yeah, all exciting stuff | 13:34 |
| NobodyCam | :) | 13:34 |
| openstackgerrit | Merged openstack/ironic-python-agent: Move _get_agent_params() to a common place https://review.openstack.org/163078 | 13:36 |
| jjohnson2 | I wager that openbmcs ipmi stack would be cooler at it... though I would laugh if pyghmi were it's ipmi target software (doubtful) | 13:42 |
| jjohnson2 | I relayed the OpenBMC stuff to our firmware guys, I only visit their world, I don't ever have time to stay | 13:42 |
| *** lazy_prince is now known as killer_prince | 13:42 | |
| *** yuanying has joined #openstack-ironic | 13:43 | |
| *** yuanying has quit IRC | 13:48 | |
| *** r-daneel has joined #openstack-ironic | 13:55 | |
| openstackgerrit | Lucas Alvares Gomes proposed openstack/ironic-python-agent: Add support for root device hints https://review.openstack.org/163079 | 13:58 |
| jjohnson2 | now that I looked at OpenBMC, it doesn't actually yet have any IPMI in it, fascinating | 13:59 |
| jjohnson2 | or else I can't find it anywhere but their block diagram | 13:59 |
| *** jgrimm is now known as zz_jgrimm | 14:00 | |
| jroll | jjohnson2: as I understand it, it just builds a linux image | 14:01 |
| *** jjohnson2 has quit IRC | 14:01 | |
| jroll | which makes me want to use an htp api rather than ipmi | 14:01 |
| jroll | but I digress because you left | 14:01 |
| jroll | http* | 14:01 |
| *** jjohnson2 has joined #openstack-ironic | 14:01 | |
| jjohnson2 | jroll, yeah, but they currently include the i2c stuff and some similar... guess I'll see where it goes.. I'm interested in what the IPMI target implementation looks like | 14:02 |
| openstackgerrit | Steven Hardy proposed openstack/ironic: Fix wrong chown command in deployment guide https://review.openstack.org/163443 | 14:05 |
| jroll | jjohnson2: you dropped for a minute, but it makes me want to use http rather than ipmi :) | 14:07 |
| *** mdbooth has quit IRC | 14:09 | |
| jjohnson2 | jroll, well, the problem is not so much about the protocol, it's the lack of specificity of non-ipmi protocols that bug me | 14:13 |
| jjohnson2 | jroll, e.g. '0 2 0' always means 'turn *this* thing off', no matter who the implementer is | 14:14 |
| jjohnson2 | jroll, the other attempts at standards seem to go out of their way to preserve the ability for a vendor to have their stuff look like a precious snowflake... | 14:14 |
| *** mdbooth has joined #openstack-ironic | 14:14 | |
| jroll | jjohnson2: I see. idk much about this space :) | 14:14 |
| jjohnson2 | I have an unhealthy interest in this space | 14:15 |
| jroll | heh | 14:16 |
| jjohnson2 | I also propose that in the wild, if IPMI didn't have an authcode on RAKP2 and also localized key to BMC UUID, then it would actually be more secure in practice than usual TLS access to BMCs | 14:16 |
| jjohnson2 | not because TLS is less secure in theory, but because in practice admins add 'verify_ssl=False' directives everywhere rather than do a cert infrastructure | 14:17 |
| jroll | indeed :/ | 14:17 |
| jjohnson2 | with SNMPv3 USM and IPMI (as modified), then it naturally verifies the server (though there is a TLS mode that acts similarly that no one uses) | 14:18 |
| jjohnson2 | and now I'll let the hook pull me off the stage and let people move on with interesting topics | 14:18 |
| dmellado | ya | 14:21 |
| jroll | jjohnson2: as if we have interesting things to talk about :) | 14:23 |
| jjohnson2 | jroll, just grading on a curve | 14:23 |
| jroll | :P | 14:23 |
| *** absubram has quit IRC | 14:24 | |
| *** mtanino has joined #openstack-ironic | 14:36 | |
| *** ChuckC_ has joined #openstack-ironic | 14:43 | |
| jlvillal | Good morning Ironic :) | 14:43 |
| *** ChuckC has quit IRC | 14:44 | |
| devananda | morning, all | 14:44 |
| * jlvillal still adapting to daylight savings time. Unlike jroll who seems to be on east coast time ;) | 14:46 | |
| *** ChuckC_ has quit IRC | 14:49 | |
| NobodyCam | morning devananda | 14:50 |
| NobodyCam | morning jlvillal | 14:50 |
| vdrok | morning everyone | 14:51 |
| vdrok | there is a question about this patch - https://review.openstack.org/#/c/151951/5 | 14:51 |
| NobodyCam | jroll: mind if I approve 158644 or would you like to have a look see? | 14:51 |
| vdrok | here https://review.openstack.org/#/c/151951/5/ironic/common/glance_service/service_utils.py exception is thrown | 14:51 |
| NobodyCam | morning vdrok :) | 14:51 |
| jlvillal | morning: NobodyCam, vdrok, devananda, jroll, jjohnson2, dmellado, lucasagomes and anyone else here.... | 14:51 |
| vdrok | morning NobodyCam, jlvillal | 14:52 |
| lucasagomes | heh morning jlvillal | 14:52 |
| dmellado | motning! | 14:52 |
| jjohnson2 | jlvillal, Woo I feel privileged not being one of 'anyone else here....' | 14:52 |
| NobodyCam | :) | 14:52 |
| jjohnson2 | and morning | 14:52 |
| vdrok | morning lucasagomes, devananda, dmellado, jjohnson2 | 14:52 |
| jjohnson2 | vdrok, morning | 14:52 |
| jlvillal | jjohnson2: :) | 14:53 |
| vdrok | should function like is_glance_image throw an exception, how do you think? | 14:53 |
| *** anderbubble has joined #openstack-ironic | 14:53 | |
| jjohnson2 | vdrok, I think all functions should only throw exceptions and never ever return or otherwise cleanly exit. Life is more interesting that way. | 14:54 |
| devananda | vdrok: is there a benefit to the UUID checking here? | 14:54 |
| devananda | vdrok: as opposed to catching the exception when attempting to request the image from glance? | 14:54 |
| vdrok | devananda, just to fail earlier | 14:54 |
| devananda | vdrok: that's wasteful | 14:55 |
| *** anderbubble has quit IRC | 14:55 | |
| devananda | vdrok: just because it is a valid UUID does not mean the glance call is going to succeed or fail | 14:55 |
| *** anderbubble has joined #openstack-ironic | 14:55 | |
| vdrok | devananda, yes, thats right | 14:55 |
| *** zz_jgrimm is now known as jgrimm | 14:55 | |
| devananda | vdrok: if the user has claimed "this is a glance ref", then we should try it. actually see if glance knows about it // we have access to the href | 14:56 |
| devananda | vdrok: also, note that, just because ironic can access the image once, does not mean ironic can access the image a second time -- maybe it gets deleted | 14:56 |
| devananda | vdrok: so early checking is never a replacement for proper error handling later on | 14:56 |
| devananda | and it's mostly just additional code maintenance burden | 14:56 |
| devananda | vdrok: what if glance allows images to be refereced by name instead of UUID ? | 14:57 |
| vdrok | devananda, for now it seems it's not the case | 14:57 |
| vdrok | devananda, but yeah, then it would have to be changed again | 14:58 |
| devananda | yup | 14:59 |
| jroll | NobodyCam: do what you will :) | 14:59 |
| jroll | morning devananda, vdrok, jlvillal :) | 14:59 |
| devananda | vdrok: IMO this check shouldn't be added. If we're going to add any early checking, I would actually verify that Ironic can fetch the glance image metadata from glance | 14:59 |
| vdrok | morning jroll | 15:00 |
| NobodyCam | :) approved | 15:00 |
| rloo | devananda, vdrok: we have validate()s that do some checking beforehand. And we're going (I think) to have VALIDAT* states. | 15:02 |
| *** anderbubble has left #openstack-ironic | 15:02 | |
| rloo | devananda, vdrok: is our existing validate() a poor-man's validate? | 15:02 |
| rloo | devananda: since that method already exists and is doing some checking -- do you think it shouldn't exist, or that it should do minimal checking? | 15:02 |
| rloo | devananda: I saw another patch that prechecks that a temp file can be created... and someone made a similar comment about why precheck. I am wondering where to 'draw the line'. | 15:04 |
| vdrok | rloo, devananda, I think it should exist in some form, as we need to know which fields should be present in e.g. instance_info | 15:04 |
| *** jcoufal has quit IRC | 15:05 | |
| vdrok | the set of required fields is different for glance and nonglance images | 15:05 |
| *** jcoufal has joined #openstack-ironic | 15:05 | |
| devananda | rloo: the ENROLL -> VALIDAT* -> MANAGEABLE state transition has nothing to do with glance | 15:05 |
| devananda | rloo: it is all about "has the user provided enough information to Ironic for Ironic to MANAGE the hardware?" | 15:05 |
| devananda | which means node.driver_info | 15:05 |
| devananda | not instance_info | 15:05 |
| rloo | devananda: oh right. thx for pointing that out. | 15:06 |
| devananda | rloo: yea, I have already commented on the temp file review, I think. that should be a one-time startup check | 15:06 |
| devananda | NobodyCam: ^ | 15:06 |
| devananda | it's reasonable for a driver, during __init__ to ensure that it can function properly | 15:06 |
| *** pas-ha has quit IRC | 15:06 | |
| devananda | we dont - bu tI think we should - have some init checking in the pxe driver, eg. to ensure that the tftpdir setting is correct and writable, and so on | 15:06 |
| devananda | I could also see the is_glance_image() function querying glance to find out if this is ACTUALLY a glance image | 15:07 |
| devananda | however, doing that, we'd need to audit every place it's called and make sure we're not going to hammer the glance API with validation requests | 15:07 |
| *** stendulker has joined #openstack-ironic | 15:08 | |
| devananda | vdrok: yes - some validation should be done. Did the user provide the correct inputs for instance_info? -- this is really up to each driver | 15:08 |
| devananda | vdrok: and it should be done by driver.deploy.validate() | 15:08 |
| vdrok | devananda, rloo, so it seems that leaving is_glance_image as is is fine | 15:10 |
| *** rameshg87 has joined #openstack-ironic | 15:10 | |
| *** yog_ has quit IRC | 15:10 | |
| rloo | vdrok: well, I would leave it as is. But I didn't want to discourage you and part of that change didn't seem to hurt. | 15:11 |
| vdrok | rloo, that's fine :) | 15:12 |
| *** ChuckC_ has joined #openstack-ironic | 15:16 | |
| NobodyCam | devananda: the latest rev should only be checking in init. also I do not see a comment from you on that review? | 15:19 |
| devananda | ugh. yes. well - I just commented | 15:21 |
| devananda | i have commented in my mind. isn't that enough? :) | 15:22 |
| devananda | NobodyCam: one missing unit test, but IMO that change is fine | 15:22 |
| devananda | NobodyCam: i wouldn't be opposed to similar checks for other drivers that have external dependencies, eg, pxe driver making sure that CONF.tftp_root is writable | 15:23 |
| devananda | NobodyCam: so with that in mind, you might also want to rename these methods from check_tmpdir_* to just check_dir_* | 15:24 |
| * devananda write smore comments on teh review | 15:24 | |
| *** david-lyle_afk is now known as david-lyle | 15:28 | |
| rloo | devananda: wrt the client supporting versioning, do we want to put that in place before https://review.openstack.org/#/c/148804/ | 15:28 |
| rloo | devananda: so that it only supports inspect, etc, if the version specified is > 1.1 (or whatever it should be) | 15:29 |
| *** gridinv_ has joined #openstack-ironic | 15:29 | |
| rameshg87 | JoshNang: you around ? | 15:29 |
| *** mtanino is now known as mtanino_away | 15:30 | |
| *** Marga_ has joined #openstack-ironic | 15:30 | |
| NobodyCam | devananda: ack TY | 15:32 |
| *** BadCub is now known as BadCub_Eating | 15:33 | |
| *** gridinv_ has quit IRC | 15:34 | |
| devananda | NobodyCam: more comments just now | 15:34 |
| *** Marga_ has quit IRC | 15:36 | |
| JoshNang | rameshg87: o/ | 15:40 |
| jlvillal | rameshg87: About that mkfs function... :P | 15:41 |
| jlvillal | rameshg87: I think you have been asked about it like four times now :D | 15:42 |
| rameshg87 | jlvillal: yeah true :) | 15:47 |
| jlvillal | rameshg87: If you re-spin again, maybe add a TODO note about re-doing the unit testing? | 15:48 |
| rameshg87 | jlvillal: yeah sure .. will do | 15:48 |
| rameshg87 | JoshNang: hi, have some time to discuss about cleaning/zapping related thing ? | 15:49 |
| JoshNang | rameshg87: sure. nothings really changed since thurs | 15:49 |
| rameshg87 | JoshNang: yeah .. some doubts after that | 15:49 |
| rameshg87 | JoshNang: https://review.openstack.org/#/c/161453/3/ironic/drivers/modules/agent.py - get_clean_steps() | 15:50 |
| rameshg87 | JoshNang: i assume you would need to add 'interface' as 'deploy' in all the clean steps that ramdisk returns | 15:50 |
| rameshg87 | JoshNang: to make it work with your parent patch | 15:50 |
| *** Marga_ has joined #openstack-ironic | 15:51 | |
| JoshNang | either that or add it to the agent. but yeah, probably the driver | 15:51 |
| JoshNang | (this is what happens when i don't have tests yet ha) | 15:52 |
| rameshg87 | :) | 15:52 |
| rameshg87 | yeah i agree, it was just for me to see | 15:52 |
| rameshg87 | but i suppose adding at the agent has some value | 15:52 |
| rameshg87 | JoshNang: we can call in the agent as result = agent_client.get_clean_steps(task.node, ports, interface='deploy') | 15:53 |
| openstackgerrit | Merged openstack/ironic: PXE driver: Deprecate pxe_deploy_{ramdisk, kernel} https://review.openstack.org/158644 | 15:53 |
| devananda | lucasagomes: nit in your pxe options patch ^ | 15:53 |
| rameshg87 | JoshNang: and let the hardware managers decide which all clean steps belong to which all interface | 15:53 |
| *** coolsvap is now known as coolsvap|afk | 15:54 | |
| rameshg87 | JoshNang: that would mean that proliant hardware manager can decide 'create_configuration' and 'delete_configuration' belongs to interface 'raid' | 15:54 |
| JoshNang | rameshg87: hmm that's interesting | 15:54 |
| rameshg87 | JoshNang: i can poll that from the same code from AgentRaidInterface with agent_client.get_clean_steps(task.node, ports, interface='raid') | 15:54 |
| rameshg87 | JoshNang: i mean that's the case with any hardware manager :) | 15:54 |
| rameshg87 | not only proliant | 15:54 |
| JoshNang | right | 15:55 |
| *** rwsu-afk is now known as rwsu | 15:55 | |
| JoshNang | the heartbeats would still come to the agent driver | 15:55 |
| JoshNang | but we already discussed solutions for that | 15:55 |
| stendulker | devananda: Hi, there were 2 ilo defects that we wanted for kilo-3 milestone, One has been updated. Did you have look at other one https://bugs.launchpad.net/ironic/+bug/1418327 | 15:56 |
| openstack | Launchpad bug 1418327 in Ironic "pxe_ilo and iscsi_ilo driver sets capabilities:boot_mode in node property if there is none" [High,In progress] - Assigned to Shivanand Tendulker (shivanand-tendulker) | 15:56 |
| lucasagomes | devananda, in a call, will take a look soonish | 15:56 |
| lucasagomes | devananda, morning btw | 15:56 |
| rameshg87 | JoshNang: it would come to agent driver, but again we know what the last step and where it belongs | 15:56 |
| *** BadCub_Eating is now known as BadCub | 15:56 | |
| JoshNang | right | 15:56 |
| rameshg87 | JoshNang: add a temporary hook in place only for create_configuration | 15:57 |
| lucasagomes | devananda, oh, I will add a patch putting the node.uuid there | 15:57 |
| rameshg87 | JoshNang: and then call update_raid_info with that | 15:57 |
| rameshg87 | JoshNang: does that seem alright ? | 15:57 |
| JoshNang | it's a good starting point | 15:57 |
| rameshg87 | okay. i will do some experiments today with your patch and let you know .. :) | 15:58 |
| JoshNang | awesome | 15:59 |
| JoshNang | i'll add some tests and make that patch correct (and hopefully get execute clean steps merged..) | 15:59 |
| rameshg87 | JoshNang: okay .. | 16:00 |
| rameshg87 | JoshNang: so you are going to add something like interface parameter and would end up calling like result = agent_client.get_clean_steps(task.node, ports, interface='deploy'), right ? | 16:01 |
| *** purp_away has quit IRC | 16:01 | |
| JoshNang | right | 16:01 |
| rameshg87 | JoshNang: that would help in separating out what belongs to deploy and what belongs to raid | 16:01 |
| rameshg87 | JoshNang: okay | 16:01 |
| rameshg87 | great .. | 16:01 |
| rameshg87 | i would make changes in proliant hardware manager accordingly and then try | 16:01 |
| *** purp_2 has joined #openstack-ironic | 16:01 | |
| *** purp_2 is now known as purp | 16:02 | |
| rameshg87 | anyone has thoughts on this: https://github.com/openstack/ironic/blob/master/ironic/api/controllers/v1/port.py#L293-L295 | 16:07 |
| rameshg87 | i think this should be done after acquiring a lock on the node | 16:07 |
| *** stendulker has quit IRC | 16:10 | |
| *** stendulker_ has joined #openstack-ironic | 16:11 | |
| openstackgerrit | Lucas Alvares Gomes proposed openstack/ironic: Add node UUID to deprecated log message https://review.openstack.org/163509 | 16:11 |
| lucasagomes | devananda, ^ | 16:11 |
| *** eglute has quit IRC | 16:13 | |
| *** Ng has quit IRC | 16:13 | |
| *** wendar_ has joined #openstack-ironic | 16:14 | |
| *** comstud has quit IRC | 16:14 | |
| *** jroll has quit IRC | 16:14 | |
| *** wendar has quit IRC | 16:14 | |
| *** rainya has quit IRC | 16:14 | |
| *** russell_h has quit IRC | 16:14 | |
| *** zigo_ has quit IRC | 16:14 | |
| *** Isotopp has quit IRC | 16:14 | |
| *** zigo_ has joined #openstack-ironic | 16:14 | |
| *** russell_h has joined #openstack-ironic | 16:15 | |
| *** eglute has joined #openstack-ironic | 16:15 | |
| *** Isotopp has joined #openstack-ironic | 16:15 | |
| *** Isotopp has quit IRC | 16:15 | |
| *** Isotopp has joined #openstack-ironic | 16:15 | |
| *** jroll has joined #openstack-ironic | 16:15 | |
| *** rainya has joined #openstack-ironic | 16:15 | |
| *** russell_h has quit IRC | 16:15 | |
| *** russell_h has joined #openstack-ironic | 16:15 | |
| *** comstud has joined #openstack-ironic | 16:15 | |
| *** jroll has quit IRC | 16:16 | |
| *** jroll has joined #openstack-ironic | 16:16 | |
| *** jmccrory has joined #openstack-ironic | 16:19 | |
| *** Ng has joined #openstack-ironic | 16:20 | |
| stendulker_ | lucasgomes: Hi, A query on boot_option capability | 16:24 |
| stendulker_ | lucasgomes: Why are we reading boot_option capability from both instance_info and node properties? And not from just instance_info? | 16:25 |
| *** ukalifon1 has quit IRC | 16:26 | |
| *** ijw has joined #openstack-ironic | 16:29 | |
| lucasagomes | stendulker_, for local boot? where's it that? | 16:31 |
| lucasagomes | stendulker_, instance_info will only be populated after deployment is triggered | 16:31 |
| lucasagomes | stendulker_, that's the nova driver that populates it into ironic | 16:31 |
| lucasagomes | where on the properties/capabilities we need to read it before to validate if that's a valid capability | 16:32 |
| JayF | I just ressurected https://review.openstack.org/#/c/155117/1 to get agent_ssh check jobs running on devstack | 16:32 |
| lucasagomes | some drivers doesn't support local boot | 16:32 |
| JayF | to prevent us being broken like we were during the rogue midcycle | 16:32 |
| stendulker_ | lucasgomes: https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/pxe.py#L478 we use instance_info | 16:32 |
| lucasagomes | so on validate() it should fail if it's set the wrong thing | 16:32 |
| *** mtanino_away has quit IRC | 16:32 | |
| stendulker_ | lucasgomes: https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/pxe.py#L511 uses node capabilities | 16:33 |
| lucasagomes | stendulker_, yes, what I said | 16:33 |
| lucasagomes | stendulker_, the first link is part of deployment | 16:33 |
| lucasagomes | we need to check if the machine should be deployed with local boot | 16:34 |
| lucasagomes | the second one is validate() that happens before deployment | 16:34 |
| lucasagomes | so we need to check if the driver supports localboot in case it's set on the node properties | 16:34 |
| lucasagomes | at the point instance_info won't be populated yet | 16:34 |
| stendulker_ | lucasgomes: Ok. Got it | 16:35 |
| lucasagomes | stendulker_, the second one is just to make sure we don't fail at deployment time | 16:35 |
| lucasagomes | so we fail before :) | 16:35 |
| lucasagomes | which is good | 16:35 |
| stendulker_ | lucasgomes: I assumed validate call happens only once. But nova does make acall to validate before initiating deploy as well | 16:35 |
| lucasagomes | stendulker_, yup | 16:35 |
| lucasagomes | stendulker_, also you can call validate via the API as well | 16:35 |
| lucasagomes | and that should tell the operator like: hey this driver doesn't support this capabilitiy | 16:36 |
| stendulker_ | lucasgomes: so for all capabilities in deployment phase we should use instance_info and in validate we should use node properties. | 16:37 |
| stendulker_ | lucasgomes: thank you for the help. | 16:37 |
| lucasagomes | stendulker_, yeah, that's because the nova driver is the one that actually populate the instance_info with the capabilities | 16:38 |
| lucasagomes | stendulker_, np | 16:38 |
| lucasagomes | stendulker_, btw I just +2'd the iLO patch you pointed to me this morning | 16:38 |
| lucasagomes | lgtm :) | 16:38 |
| stendulker_ | lucasgomes: thank you :) | 16:38 |
| *** rameshg87 is now known as rameshg87-dinner | 16:49 | |
| *** andreykurilin_ has joined #openstack-ironic | 16:51 | |
| *** openstack has joined #openstack-ironic | 16:55 | |
| *** andreykurilin_ has quit IRC | 16:56 | |
| NobodyCam | devananda: should we add a link to https://wiki.openstack.org/wiki/Ironic/CoreTeam on https://wiki.openstack.org/wiki/Ironic | 16:56 |
| *** romcheg1 has quit IRC | 16:57 | |
| NobodyCam | I ask because I had the core team page open in a tab and I've forgotten why I had it open :-p | 16:58 |
| *** BadCub is now known as BadCub_Away | 17:00 | |
| *** stendulker_ has quit IRC | 17:02 | |
| *** spandhe has joined #openstack-ironic | 17:07 | |
| openstackgerrit | Ramakrishnan G proposed openstack/ironic-python-agent: SyncCommandResult should store actual exception https://review.openstack.org/163524 | 17:10 |
| *** harlowja_away is now known as harlowja_ | 17:14 | |
| *** mgagne_PHL is now known as mgagne | 17:14 | |
| *** romcheg has joined #openstack-ironic | 17:18 | |
| openstackgerrit | Ramakrishnan G proposed openstack/ironic-python-agent: os.path.exists should be mocked in test case https://review.openstack.org/163529 | 17:19 |
| *** chlong has quit IRC | 17:21 | |
| rloo | NobodyCam: are we ok with a commit that just has 'Fix some typos'? | 17:25 |
| rloo | NobodyCam: https://review.openstack.org/#/c/163422/ | 17:25 |
| rloo | NobodyCam: maybe I'm just nit'ing? | 17:26 |
| devananda | NobodyCam: oh - yes, we should | 17:26 |
| NobodyCam | rloo: I was okay with that as it litteraly fixed two typos :) | 17:27 |
| *** wendar_ is now known as wendar | 17:28 | |
| jroll | rloo: you have the power to change the commit message :) | 17:28 |
| rloo | NobodyCam: it doesn't even say 'two' typos. it says 'some'. | 17:29 |
| rloo | jroll: yeah, i have the power but i think i'll just skip that patch ;) | 17:29 |
| jroll | :/ | 17:29 |
| NobodyCam | :-p rloo I took some from this page: http://law.marquette.edu/facultyblog/2014/07/20/commonly-confused-words-a-couple-a-few-some-several-or-many/ | 17:31 |
| openstackgerrit | Jim Rollenhagen proposed openstack/ironic: Fix two typos https://review.openstack.org/163422 | 17:31 |
| jroll | fixed. | 17:31 |
| NobodyCam | which says it could be as little as a few (two) or more... so some worked for me there | 17:31 |
| NobodyCam | lol | 17:31 |
| jroll | NobodyCam: +a that pls | 17:31 |
| jroll | get it out of the queue | 17:31 |
| JayF | typo-only things only need one +2A per the recent list thread | 17:32 |
| jroll | oh | 17:32 |
| jroll | right | 17:32 |
| jroll | +a'd | 17:32 |
| jroll | I get upset when we spend more time talking about fixing a thing than it takes to fix the thing :| | 17:33 |
| NobodyCam | lol ok so now it has 2 +a's | 17:34 |
| NobodyCam | lol | 17:34 |
| NobodyCam | :) | 17:34 |
| jroll | must be a good patch :D | 17:34 |
| jlvillal | NobodyCam: Dumb question what does 'a' in '+a' stand for? I know it is the Workflow field. But curious what 'a' means | 17:35 |
| jroll | approved | 17:35 |
| * NobodyCam can't wait to have real bandwidth again (installer will be here tomorrow) (happydance) | 17:35 | |
| jroll | some people say +W, some people say +3 | 17:35 |
| jlvillal | jroll: Ah, thanks :) | 17:35 |
| NobodyCam | yep | 17:35 |
| jroll | hyve just announced a new ocp platform called honey badger, lol | 17:35 |
| NobodyCam | lol | 17:35 |
| NobodyCam | better then honey booboo | 17:36 |
| NobodyCam | lol | 17:36 |
| jroll | hahaha | 17:36 |
| jroll | you've seen the honey badger video no? | 17:36 |
| NobodyCam | ?? like from a few years ago? | 17:36 |
| jroll | yeah | 17:37 |
| jroll | just making sure :) | 17:37 |
| jroll | hyve don't give a... | 17:37 |
| NobodyCam | lol - i still like (i don't know why) the broken goat | 17:38 |
| jroll | lol | 17:38 |
| NobodyCam | https://www.youtube.com/watch?v=Q8ZpW0cDc5U | 17:38 |
| jroll | amazing | 17:39 |
| *** gridinv_ has quit IRC | 17:39 | |
| *** igordcard_ has joined #openstack-ironic | 17:42 | |
| *** devlaps has joined #openstack-ironic | 17:42 | |
| *** jmccrory has quit IRC | 17:42 | |
| NobodyCam | lol | 17:43 |
| NobodyCam | I lol every time I watch that one.. :-p | 17:43 |
| devananda | anyone familiar with this bug? https://bugs.launchpad.net/ironic/+bug/1427597 | 17:44 |
| openstack | Launchpad bug 1427597 in Ironic "destroy_port fails to delete the port even if the node is locked by the same process. " [Medium,In progress] - Assigned to Nisha Agarwal (agarwalnisha1980) | 17:44 |
| devananda | seems like dtantsur and nisha were working on it | 17:44 |
| NobodyCam | devananda: I seem to recall Nisha hitting that one. | 17:45 |
| jroll | devananda: somewhat, why | 17:45 |
| rameshg87-dinner | devananda: yes, i am :) | 17:46 |
| devananda | rameshg87-dinner: hi! I just commented on nisha's patch | 17:46 |
| devananda | it would be helpful to have a description of the sequence of events that creates this issue | 17:46 |
| *** rameshg87-dinner is now known as rameshg87 | 17:46 | |
| devananda | not just "during discovery" -- but why is the node lock held by one thread, while the client is issuing a delete request to the REST API? | 17:47 |
| devananda | that's the part I'm really confused by | 17:47 |
| devananda | it seems like the client must be issuing parallel requests // issuing the second request before the first one is done? or am I missing something | 17:47 |
| jroll | iirc the node is locked for the entirety of discovery? | 17:48 |
| rameshg87 | devananda: yeah, i agree about description | 17:48 |
| rameshg87 | devananda: yeah the node is locked during discovery which causes the reservation field to be set | 17:48 |
| devananda | huh | 17:48 |
| devananda | but discovery is async | 17:48 |
| devananda | there's an agent that runs on the node. it might take some time to introspect, or what ever | 17:49 |
| rameshg87 | devananda: yeah but discovery needs to alter the properties of the node hence it needs to acquire the lock | 17:49 |
| devananda | what happens if, while the agent is doing its thing, I migrate conductors? | 17:49 |
| devananda | the lock cant be held for the whole duration of discovery... it needs to be taken while the conductor is *doing* something, then released | 17:49 |
| rameshg87 | yeah..but atleast during updating the node properties, we should lock the node, right ? | 17:50 |
| devananda | rameshg87: conductor.upddate_node already does that | 17:50 |
| jroll | devananda: isn't nisha working on oob inspection? | 17:50 |
| devananda | jroll: oh, right. | 17:50 |
| jroll | :) | 17:51 |
| devananda | jroll: in the conductor, or in another service ? | 17:51 |
| devananda | for oob inspection driven by the conductor, I can see how this error would occur. but the proposed patch doesnt address it. | 17:51 |
| rloo | JayF, jroll: I think the typo thing needing only one +2 +A was for non-user-facing typos ;) | 17:51 |
| rameshg87 | jroll: devananda: it's within the conductor, oob inspection | 17:51 |
| devananda | rameshg87: so why does it need a new RPC call? | 17:52 |
| jroll | devananda: in the driver. I agree this doesn't make sense to me while looking at line 50 here https://review.openstack.org/#/c/151596/29/ironic/drivers/modules/ilo/inspect.py | 17:52 |
| jroll | OH | 17:52 |
| jroll | wait. | 17:52 |
| *** mtanino has joined #openstack-ironic | 17:52 | |
| jroll | devananda: so on the patch in question... | 17:52 |
| jroll | it removes the lock check from the dbapi layer | 17:52 |
| jroll | and adds one that did not exist for the case where the API is used to destroy the port | 17:53 |
| devananda | OH | 17:53 |
| rameshg87 | yeah | 17:53 |
| jroll | now as to why ilo/inspect uses dbapi directly, that's a different problem :) | 17:53 |
| rameshg87 | with the lock check in the db layer for destroy_port, no one can delete the port when they themselves have acquired the lock | 17:53 |
| devananda | rameshg87: ok, could you update the commit message to clarify all this? | 17:53 |
| *** achanda has joined #openstack-ironic | 17:54 | |
| rameshg87 | devananda: yeah sure .. | 17:54 |
| *** anderbubble_ has joined #openstack-ironic | 17:54 | |
| devananda | thanks much | 17:55 |
| * devananda runs to a meeting | 17:55 | |
| jroll | "rackspace is building a new server based on openpower" | 17:56 |
| *** anderbubble_ is now known as anderbubble | 17:56 | |
| rameshg87 | devananda: that brings me to a question | 17:56 |
| rameshg87 | devananda: jroll: shouldn't this also be done from conductor after acquiring the lock ? https://github.com/openstack/ironic/blob/master/ironic/api/controllers/v1/port.py#L284-L298 | 17:57 |
| *** athomas has quit IRC | 17:57 | |
| jroll | rameshg87: that should probably be in the conductor, but may not need a lock. I think it's fine for now | 17:57 |
| rameshg87 | jroll: i assume we *should not be* able to delete ports during a deploy | 17:59 |
| jroll | rameshg87: that's create port no? | 18:00 |
| rameshg87 | jroll: but it *should be* okay to create new ports during something like a deploy | 18:00 |
| rameshg87 | jroll: yeah | 18:00 |
| jroll | rameshg87: yeah, indeed | 18:00 |
| jroll | should be. | 18:00 |
| *** vdrok is now known as vdrok_afk | 18:00 | |
| *** tiagogomes_ has quit IRC | 18:01 | |
| *** jmccrory has joined #openstack-ironic | 18:08 | |
| *** ifarkas has quit IRC | 18:08 | |
| *** Nisha has joined #openstack-ironic | 18:09 | |
| openstackgerrit | Ramakrishnan G proposed openstack/ironic: Remove reservation check for port destroy in db https://review.openstack.org/161861 | 18:11 |
| rameshg87 | devananda: jroll: does it sound better now ? ^^^ | 18:11 |
| *** kbyrne has quit IRC | 18:13 | |
| jroll | rameshg87: yeah, commit message lgtm. will review :) | 18:14 |
| *** kbyrne has joined #openstack-ironic | 18:14 | |
| *** anderbubble has left #openstack-ironic | 18:18 | |
| *** absubram has joined #openstack-ironic | 18:20 | |
| openstackgerrit | Nisha Agarwal proposed openstack/ironic: iLO implementation for hardware inspection https://review.openstack.org/151596 | 18:23 |
| lucasagomes | folks I will call it a day | 18:26 |
| lucasagomes | have a good night! | 18:26 |
| *** lucasagomes is now known as lucas-hungry | 18:26 | |
| *** lucas-hungry is now known as lucas-dinner | 18:26 | |
| jlvillal | lucas-dinner: Good night! | 18:28 |
| openstackgerrit | Nisha Agarwal proposed openstack/ironic: iLO implementation for hardware inspection https://review.openstack.org/151596 | 18:28 |
| Nisha | devananda, and other core reviewers, request reviews on https://review.openstack.org/151596 | 18:30 |
| Nisha | even for https://review.openstack.org/155900 | 18:32 |
| *** jcoufal has quit IRC | 18:32 | |
| NobodyCam | night lucas-dinner | 18:37 |
| rameshg87 | jroll: thanks :) | 18:38 |
| *** jmccrory has quit IRC | 18:39 | |
| *** jmccrory has joined #openstack-ironic | 18:39 | |
| *** pelix has quit IRC | 18:43 | |
| *** jistr has quit IRC | 18:46 | |
| *** morganfainberg has joined #openstack-ironic | 18:56 | |
| *** mtanino has quit IRC | 18:57 | |
| *** mtanino has joined #openstack-ironic | 18:57 | |
| *** Marga_ has quit IRC | 18:58 | |
| *** jmccrory has quit IRC | 19:02 | |
| openstackgerrit | Nisha Agarwal proposed openstack/ironic: Inpects few more scheduling properties with inspection https://review.openstack.org/163572 | 19:13 |
| openstackgerrit | Merged openstack/ironic: Fix wrong chown command in deployment guide https://review.openstack.org/163443 | 19:18 |
| openstackgerrit | Nisha Agarwal proposed openstack/ironic: Inpects hardware and gets associated some of the capabilities. https://review.openstack.org/163572 | 19:20 |
| jgrimm | bah, still having no luck getting IPA to work. deploy 'succeeds' now, but none of the instance images i've tested actually boot. :( | 19:26 |
| jgrimm | i did run into issue where i had an MBR already on the disk... and writing the config drive at the end of the disk barfed, because it was past mbr limits. i'll bug that. | 19:26 |
| *** achanda has quit IRC | 19:27 | |
| jgrimm | tried instance image of cirros img (per recommendation) and coreos iso for kicks. deploy 'succeeds' and reboot happens, but no booting | 19:28 |
| jroll | what's the console show? | 19:29 |
| jgrimm | complete blank | 19:29 |
| jroll | hrm | 19:30 |
| jgrimm | i'm surprised the bios didn't spit out some angry message at least.. but i saw that behavior when i was deploying pxe_ipmitool localboot too before i figured out grub hadn't been layed down | 19:31 |
| jroll | right... hmm | 19:31 |
| jroll | you see bios and stuff yeah? | 19:31 |
| jroll | maybe grub it pointed at the wrong tty | 19:31 |
| *** jmccrory has joined #openstack-ironic | 19:31 | |
| jgrimm | i don't think its even booting. or at least never makes it to dhcp'ing | 19:32 |
| jgrimm | as evidence of 'something' interesting happening | 19:33 |
| jroll | oh, this is pxe_ipa | 19:33 |
| jroll | or pxe_ipmitool with ipa | 19:33 |
| jgrimm | this is with agent_ipmitool | 19:34 |
| jgrimm | i haven't tried pxe_ipmitool with ipa deploy image combo yet | 19:35 |
| jroll | well it wouldn't be dhcp'ing until it boots up then | 19:36 |
| jgrimm | yep | 19:36 |
| *** achanda has joined #openstack-ironic | 19:38 | |
| jgrimm | i'm probably still doing 'something' wrong, but not seeing any clues to what might be at fault. i suppose i should try to actually boot the coreos image somewhere else and verify it boots. | 19:39 |
| jroll | I do know for a fact the cirros image will boot | 19:39 |
| jroll | assuming your using the disk image | 19:39 |
| jgrimm | jroll, yep: | cb067306-1140-4c41-85e0-9849d077d37c | cirros-0.3.2-x86_64-disk | qcow2 | bare | 13167616 | active | 19:41 |
| jroll | yeah, that should boot. I bet the console= kernel argument is wrong for your hardware | 19:41 |
| jgrimm | jroll, ok, let me look at that, but odd that networking never comes up.. even if i don't get a console | 19:41 |
| NobodyCam | `brb | 19:41 |
| jroll | jgrimm: well, that will depend on dhcp working properly and stuff, idk | 19:42 |
| jgrimm | jroll, yep! | 19:42 |
| jroll | which we can't figure out without logs :P | 19:42 |
| jgrimm | :) indeed | 19:42 |
| rameshg87 | jgrimm: one thing that happened to me recently | 19:44 |
| rameshg87 | jgrimm: does your instance image has the bootable flag set for the root partition ? | 19:44 |
| rameshg87 | jgrimm: i have experience that bios doesn't boot from mbr if it doesn't find atleast one bootable partition | 19:45 |
| rameshg87 | <some bios> rather | 19:45 |
| jgrimm | rameshg87, i recall seeing that bug.. that would be interesting | 19:45 |
| rameshg87 | jgrimm: you can immediately check that | 19:46 |
| rameshg87 | jgrimm: down the glance image, run fdisk -l <> on it | 19:46 |
| rameshg87 | jgrimm: and check if fdisk shows atleast one partition as bootable | 19:46 |
| jgrimm | rameshg87, cool. thanks for the idea | 19:46 |
| openstackgerrit | Ramakrishnan G proposed openstack/ironic: Add whole disk image support in iscsi_ilo driver https://review.openstack.org/163589 | 19:47 |
| rameshg87 | jgrimm: and btw cirros image doesn't actually boot on my hardware | 19:47 |
| jgrimm | rameshg87, ohh... for that reason? | 19:47 |
| rameshg87 | jgrimm: it has limited set of kernel drivers for the actual hardware | 19:47 |
| jgrimm | ahh.. ok | 19:47 |
| rameshg87 | jgrimm: in my understanding, cirros image is mainly for testing purposes on virtual machines | 19:48 |
| rameshg87 | jgrimm: i guess nobody qualifies it on any actual hardware | 19:48 |
| jgrimm | agreed. i'm getting same behavior with coreos iso image i've built too, but that doesn't mean much more than cirros | 19:49 |
| jgrimm | or less even | 19:49 |
| openstackgerrit | Ramakrishnan G proposed openstack/ironic: iscsi_ilo driver to support agent ramdisk https://review.openstack.org/162449 | 19:56 |
| *** kevinbenton has joined #openstack-ironic | 19:57 | |
| jgrimm | rameshg87, Disk cirros-0.3.2-x86_64-disk.img doesn't contain a valid partition table | 19:58 |
| jgrimm | rameshg87, Disk cirros-0.3.2-x86_64-disk.img doesn't contain a valid partition table | 19:58 |
| jgrimm | so that does sound possibly wonky that my system doesn't like, but seems to work for others. | 20:00 |
| *** pas-ha has joined #openstack-ironic | 20:01 | |
| rameshg87 | jgrimm: even i have one, let me try | 20:02 |
| jgrimm | rameshg87, tx! | 20:03 |
| rameshg87 | jgrimm: it's qcow2 | 20:03 |
| rameshg87 | jgrimm: did you uncompress it | 20:03 |
| rameshg87 | jgrimm: it shows for me | 20:03 |
| rameshg87 | jgrimm: http://paste.openstack.org/show/191685/ | 20:04 |
| rameshg87 | jgrimm: it's marked as bootable, i think it should be too in your case | 20:04 |
| rameshg87 | jgrimm: so that might not be the problem :) | 20:04 |
| jgrimm | oh sorry, yeah, i need to convert to raw | 20:04 |
| rameshg87 | jgrimm: i have never been able to boot cirros image on actual hardware btw | 20:05 |
| rameshg87 | jgrimm: i take something like: https://cloud-images.ubuntu.com/trusty/current/trusty-server-cloudimg-amd64-disk1.img | 20:05 |
| rameshg87 | jgrimm: for quick testing ... | 20:06 |
| jgrimm | rameshg87, cool, i was about to ask for your methodology | 20:06 |
| rameshg87 | jgrimm: it comes with cloud-init in it, so you should be able to do your normal login with configdrive as well | 20:06 |
| jgrimm | heh, i'd be happy to get to a login | 20:07 |
| rameshg87 | :) | 20:07 |
| * rameshg87 feels the same when i get a login prompt :) | 20:08 | |
| * rameshg87 goes to sleep | 20:08 | |
| rameshg87 | good night ironic | 20:08 |
| jgrimm | rameshg87, thanks for ideas rameshg87, good night | 20:08 |
| *** rameshg87 has left #openstack-ironic | 20:09 | |
| *** Marga_ has joined #openstack-ironic | 20:20 | |
| *** Nisha has quit IRC | 20:28 | |
| *** ChuckC_ has quit IRC | 20:29 | |
| jlvillal | I would appreciate reviews on: https://review.openstack.org/#/c/163600/ httpretty 0.8.8 breaks the py34 tests of python-ironicclient | 20:32 |
| jlvillal | So I am submitting change to openstack/requirements to fix it | 20:33 |
| jlvillal | This also means the gate on openstack/python-ironicclient is broken until that is fixed. | 20:33 |
| rloo | jlvillal: seems like deja vu? | 20:34 |
| jlvillal | rloo: Yep! It broke with 0.8.7. 0.8.8 was released today it seems. | 20:35 |
| *** jistr has joined #openstack-ironic | 20:35 | |
| rloo | jlvillal: any idea whether there is a bug against httpretty? or are we going to blacklist all future versions? | 20:35 |
| jlvillal | rloo: I think there is a bug about this | 20:36 |
| rloo | jlvillal: ok. | 20:36 |
| jlvillal | rloo: I think this might be the issue. Not sure though. https://github.com/gabrielfalcao/HTTPretty/issues/221 | 20:36 |
| *** ndipanov has quit IRC | 20:36 | |
| rloo | jlvillal: maybe? worth putting that link in the commit message? | 20:37 |
| jlvillal | rloo: Sure | 20:37 |
| jlvillal | rloo: Done :) | 20:39 |
| rloo | thx jlvillal | 20:39 |
| jlvillal | rloo: So... I like testtools.ExpectException in my use. Because self.assertRaises seems to much less elegant, in my opinion. | 20:42 |
| rloo | jlvillal: less elegant just in your case, or in general? | 20:43 |
| jlvillal | rloo: Less elegant when you want to check the message that the exception is generating. | 20:44 |
| jroll | jlvillal: self.assertRaises returns the exception, fwiw | 20:44 |
| jlvillal | It becomes more like. result = self.assertRaises(). Then you have to check the result with self.assertEqual | 20:44 |
| rloo | jlvillal: that would be true. but we don't tend to check the message string. that checking seems to have increased in recent tests though. i think. | 20:44 |
| jlvillal | Thus why I think it is more elegant :) | 20:44 |
| rloo | jlvillal: there was a question as to how much string checking we should be doing in the unit tests, due to locale but it wasn't clear if there was a real problem or not. | 20:46 |
| jlvillal | rloo: In this case the message is generated solely by the unit test. And verified as such. | 20:47 |
| rloo | jlvillal: my preference would be to use the assertRaises mechanism, and propose a separate something to switch to use testtools.ExpectException. I'm not fond of supporting both ways, cuz I can imagine someone will come along later and want to change them all to be one way only. | 20:47 |
| rloo | jlvillal: I'm not going to -1 you on this, it is just my opinion. | 20:47 |
| jlvillal | rloo: But yes, it would be good if we had a way to validate the messages generated some times. As it seems like the only difference in code paths is what log message they generate. So we should have a way to test that. | 20:48 |
| rloo | jlvillal: but I most likely won't +2 it either cuz I'm not sure. | 20:48 |
| rloo | jlvillal: i agree that it would be more complete to check for the expected string. | 20:48 |
| jroll | where do we get testtools from? direct dep or testr or something else? | 20:48 |
| jroll | oh, it's direct, ignore me | 20:49 |
| jlvillal | rloo: There is probably a good reason the testtools library added ExpectException in addition to adding features to self.raiseAssert | 20:49 |
| rloo | jlvillal: yes, you already gave a good reason for using it ;) | 20:49 |
| jlvillal | jlvillal: I'm not sure exactly what that good reason is :) | 20:49 |
| jlvillal | rloo: They also added the ability to do result=self.raisesAssert(). As unittest doesn't have that feature. | 20:50 |
| jlvillal | I will work on the patch, clean up some of it | 20:50 |
| rloo | jlvillal: I'm not against using testtools... and maybe lifeless can chime in ^^ | 20:52 |
| lifeless | h | 20:53 |
| lifeless | hi | 20:53 |
| lifeless | whats up? | 20:53 |
| jlvillal | rloo: Well we are using testtools :) | 20:53 |
| jlvillal | rloo: When use self.assertRaises() that is testtools | 20:54 |
| rloo | lifeless: do you have any thoughts about whether it is better to use testtools.ExpectException vs self.assertRaises()? We are using the latter in our unit tests. | 20:54 |
| jlvillal | rloo: If go back far enough in the Class chain will find that it is using testtools | 20:54 |
| rloo | jlvillal: oh, i didn't realize self.assertRaises() is from testtools also. | 20:54 |
| lifeless | theres a few different things | 20:54 |
| rloo | lifeless, did you write that stuff? you're confusing us! | 20:54 |
| lifeless | the history is this | 20:54 |
| lifeless | python 2.4's assertRaises didn't return the exception | 20:55 |
| lifeless | and there were no context managers | 20:55 |
| lifeless | so we returned the exception to make it possible to write assertions about the caught exception | 20:55 |
| lifeless | then cPythin turned assertRaises into a context manager | 20:55 |
| *** achanda has quit IRC | 20:55 | |
| lifeless | which would be an API break for testtools | 20:55 |
| lifeless | so we added ExpectException since using a context manager for this is pretty nice | 20:56 |
| *** romcheg has left #openstack-ironic | 20:56 | |
| lifeless | we also have a matcher so you can build a complex assertion around things | 20:56 |
| *** mtanino has quit IRC | 20:56 | |
| lifeless | but ExpectException is probably the most direct and easy way to write such stuff | 20:56 |
| jlvillal | lifeless: I thought using 'with ExpectException(xx, match_string)' was more elegant than 'result = self.assertRaises(); self.assertEqual(result, match_string)' | 20:56 |
| lifeless | any of them are fine IMO | 20:56 |
| * jlvillal likes the history lesson :) | 20:57 | |
| jlvillal | lifeless: Thanks for the info. | 20:57 |
| lifeless | things like https://github.com/testing-cabal/testrepository/blob/master/testrepository/tests/test_repository.py#L233 | 20:57 |
| lifeless | are pretty nice IMO | 20:57 |
| jlvillal | lifeless: That is nice :) | 20:58 |
| rloo | lifeless: ooo. almost English ;) | 20:58 |
| * jlvillal will try that... | 20:59 | |
| rloo | thanks lifeless! | 20:59 |
| *** dprince has quit IRC | 21:02 | |
| jlvillal | lifeless: So I kind of failed using assertThat() :( Can I put the call to the function that will raise an exception into the first argument? | 21:04 |
| jlvillal | I tried that, and it didn't seem like the exception was caught by assertThat. | 21:05 |
| *** r-daneel has quit IRC | 21:05 | |
| mrda | Morning Ironic | 21:05 |
| lifeless | jlvillal: the first parameter needs to be a zero-arg callable | 21:06 |
| lifeless | jlvillal: e.g. a lambda or just the function | 21:06 |
| jlvillal | lifeless: Oh :( The function I want to call has multiple args. | 21:06 |
| lifeless | jlvillal: yeah so - self.assertThat(lambda:foo(x,y,z), raises(...))) | 21:06 |
| jlvillal | lifeless: Thanks | 21:07 |
| lifeless | the thing I like about this is that the call is actually explicit and visible. The thing I don't like is that lambdas have a terrible repr() | 21:07 |
| lifeless | and are not as nice as lambdas in e.g. haskell | 21:07 |
| *** jjohnson2 has quit IRC | 21:08 | |
| *** pas-ha has quit IRC | 21:08 | |
| jlvillal | lifeless: Thanks for the info. I'm not sure which will look most readable. I will do both and stare at the code :) | 21:09 |
| lifeless | +1 | 21:09 |
| *** jistr has quit IRC | 21:10 | |
| *** yuriyz has quit IRC | 21:13 | |
| *** vdrok_afk has quit IRC | 21:13 | |
| *** vdrok_afk has joined #openstack-ironic | 21:14 | |
| *** yuriyz has joined #openstack-ironic | 21:14 | |
| jlvillal | rloo: What's your favorite? http://paste.openstack.org/show/191693/ | 21:16 |
| jlvillal | :) | 21:16 |
| rloo | jlvillal: what if you don't want/care about the exception message? Which is better? | 21:18 |
| jlvillal | rloo: In this case I like ExpectedException the best for with or without a message. | 21:19 |
| jlvillal | But I am fine using self.assertRaises() for consistency sake. | 21:19 |
| rloo | jlvillal: so wrt that particular patch, i'd prefer if you used self.assertRaises() for consistency sake. but maybe do another patch that replaces the .assertRaises() with ExpectedException in the two cases (with and w/o message) and see what people think. | 21:20 |
| jlvillal | rloo: Okay | 21:21 |
| rloo | jlvillal: that's only my opinion. | 21:21 |
| jroll | read that as "that's my only opinion" | 21:22 |
| jlvillal | rloo: It works for me. I'll try not to be too emotionally attached to my code :D | 21:22 |
| jroll | and was ready to call out rloo | 21:22 |
| jlvillal | rloo: But it is tough to not get attached to it. It's like my little precious code ;) | 21:23 |
| rloo | jroll: ha ha | 21:23 |
| rloo | jlvillal: oh my. if you are attached to your code, i'm not sure you'll like the open source community. | 21:23 |
| * jlvillal One of the reason jlvillal tries not to use terms like 'you' or 'your' when doing code reviews. | 21:23 | |
| jlvillal | rloo: :) | 21:24 |
| rloo | jlvillal: but think of the pleasure you'll get when you've convinced others that 'your' way is more elegant ;) | 21:24 |
| lifeless | jlvillal: that sample code looks like its related to https://review.openstack.org/#/c/160383/ ? | 21:24 |
| rloo | jlvillal: until the next young whipper snapper has a more elegant way... | 21:24 |
| jlvillal | rloo: True, but I'm not sure the pleasure is worth the effort :) | 21:24 |
| mrda | and sometimes that convincing is *even* on merit of the change? :-P | 21:24 |
| openstackgerrit | Merged openstack/ironic-python-agent: SyncCommandResult should store actual exception https://review.openstack.org/163524 | 21:24 |
| jlvillal | lifeless: Sort of related. yes. | 21:24 |
| lifeless | so I might be out of sync with everyone else here | 21:25 |
| jlvillal | lifeless: https://review.openstack.org/162672 is the patch. Which is updating for this change: https://review.openstack.org/161803 | 21:25 |
| jroll | lifeless: I think I agree with what you're about to say :P | 21:27 |
| jroll | we're spending an awwwful lot of time on 163524 | 21:27 |
| lifeless | jlvillal: put a review on that - I think there is a more direct fix (use a better library function) | 21:27 |
| jroll | I think it has a small benefit, but likely not worth the effort | 21:27 |
| lifeless | jroll: so what I was going to say is that pre-flight checks as a rule don't cover all the bases, better exception handling does | 21:28 |
| *** achanda has joined #openstack-ironic | 21:28 | |
| jroll | lifeless: yeah, saw your comment already :) | 21:29 |
| lifeless | jroll: is that the one you agree with ? | 21:29 |
| jlvillal | lifeless: Okay. I was just trying to handle the exceptions that could be raised in that function. Since I had updated the function.... | 21:29 |
| * jlvillal does things in reverse order at times.... | 21:29 | |
| lifeless | jlvillal: yep, and +1 on that | 21:29 |
| lifeless | jlvillal: but if we use a better function, the whole thing can become smaller :) | 21:30 |
| jroll | lifeless: I agree that this is mostly not useful, partially for that reason, though checking some basic things on init seems sane | 21:30 |
| lifeless | jroll: things that can't change at runtime are good to check at init IMO | 21:30 |
| jlvillal | lifeless: True. I guess I could work on a patch for that also. And do it together. | 21:31 |
| lifeless | jroll: I don't like the maintenance burden of double-checking things | 21:31 |
| *** jmccrory has quit IRC | 21:31 | |
| *** jmccrory1 has joined #openstack-ironic | 21:31 | |
| jroll | lifeless: yeah. though if /tmp doesn't exist then the conductor is pretty useless. so we should check that. (for example) | 21:31 |
| devananda | mrda: I believe rloo pointed out to me recently that logical names arne't working for node references under the /v1/ports/ API | 21:32 |
| mrda | devananda: :( | 21:32 |
| lifeless | jroll: well, I'll disagree - /tmp can get remounted under some circumstances while the conductor is running | 21:32 |
| lifeless | jroll: which will bork it just as effectively. Or run out of space. | 21:32 |
| mrda | I'll take a look today devananda. Is this a reported bug? Or just an observation at this stage? | 21:32 |
| devananda | mrda: just observation while testing the client changes | 21:33 |
| devananda | mrda: i have not filed a bug yet | 21:33 |
| lifeless | jroll: so I'd really rather assume it might become useless at any point and catch those exceptions | 21:33 |
| mrda | ok, cool. | 21:33 |
| lifeless | jroll: (and signal them clearly to the log or even in the object error state in the API) | 21:33 |
| mrda | I shall test it myself, confirm the observation, report a bug, and fix it. | 21:33 |
| devananda | lifeless: so, I agree with you on "handle failures well when there could be races" | 21:33 |
| jroll | lifeless: sure, I see your point. I'm fine without fixing this "bug" at all, I'm also fine with a small bit of code to do sanity checks at startup. /me checks how much code this is again | 21:33 |
| devananda | mrda: cheers :) | 21:33 |
| rloo | devananda, mrda: I looked at the port/node-name thingy briefly. the code dealing with the port is a bit ... odd. there's some stuff there wrt node id (and uuid) that ought to be cleaned up. | 21:34 |
| devananda | lifeless: but I feel like drivers have a responsibility during start-up to fail fast if they know, at that point, that their function is impaired | 21:34 |
| lifeless | I think I have two specific concerns here. One - if we're going to close the bug, we should be covering the general case, not the special case of startup. | 21:34 |
| *** ijw has quit IRC | 21:34 | |
| *** penick has joined #openstack-ironic | 21:34 | |
| lifeless | Two - I consider extra code a burden, but I know not everyone agrees on that :) | 21:34 |
| devananda | lifeless: the bug, IMHO, is really about "operator misconfigured the environment, but only found out by digging in the deploy logs" | 21:35 |
| devananda | lifeless: a driver that logs an error and fails startup solves that bug | 21:35 |
| devananda | lifeless: and it is a noticeable improvement in operator usability | 21:35 |
| mrda | rloo: thanks, I'll have a look | 21:35 |
| devananda | lifeless: it's not a replacement for proper error handling of eg, /tmp filled up, or someone changed permissions on /var/run/tmp/ironic/ or what ever | 21:36 |
| devananda | but perhaps that distinction isn't clearly stated on the bug or patch ? | 21:36 |
| lifeless | the bug appears to be about the general case | 21:37 |
| lifeless | 'Ironic drivers that utilize temp files should verify they can actually create them' | 21:37 |
| lifeless | anyhow, I won't want to obsess on this | 21:40 |
| lifeless | I agree that stopping and saying 'whoa this can't possibly work' is a useful thing to do | 21:41 |
| lifeless | I don't think its as useful as making sure the actual cause of the error reported in that bug is surfaced to the operators effectively *whenever* it occurs | 21:41 |
| lifeless | I'd be sad if the bug is closed with the fail-fast code applied but the general case not. | 21:42 |
| lifeless | fin | 21:42 |
| rloo | amen to that! | 21:45 |
| *** movielovers has joined #openstack-ironic | 21:45 | |
| *** movielovers has left #openstack-ironic | 21:45 | |
| openstackgerrit | Merged openstack/ironic: Add node UUID to deprecated log message https://review.openstack.org/163509 | 21:45 |
| openstackgerrit | Merged openstack/ironic: Fix two typos https://review.openstack.org/163422 | 21:48 |
| mrda | :) | 21:50 |
| *** spandhe has quit IRC | 21:54 | |
| openstackgerrit | Merged openstack/ironic-python-agent: os.path.exists should be mocked in test case https://review.openstack.org/163529 | 21:57 |
| *** kkoski has quit IRC | 21:57 | |
| *** arif-ali has quit IRC | 21:58 | |
| NobodyCam | brb | 21:59 |
| devananda | lifeless: +1 | 22:00 |
| openstackgerrit | John L. Villalovos proposed openstack/ironic: Update unittests and use NamedTemporaryFile https://review.openstack.org/162672 | 22:07 |
| jlvillal | rloo: lifeless: ^^^^^ with your suggestions... | 22:08 |
| *** arif-ali has joined #openstack-ironic | 22:10 | |
| lifeless | jlvillal: oh, crossed wires. Let me review this to help :) | 22:10 |
| jlvillal | jroll: devananda: Do you have any clout with people who can merge fix to unblock the gate for python-ironicclient? https://review.openstack.org/#/c/163600/ | 22:10 |
| jlvillal | httpretty again :( | 22:11 |
| jlvillal | lifeless: Thanks! | 22:11 |
| jroll | clout isn't a thing, or shouldn't be :/ | 22:11 |
| jroll | jlvillal: honestly to unbreak the gate, that change should be to python-ironicclient | 22:11 |
| jlvillal | jroll: I guess I'm being impatient. I'm sure they will get to it in time... | 22:11 |
| jroll | and then we can follow up with a change to global-reqs | 22:12 |
| jlvillal | jroll: I thought you were the one who told me I had to do global-reqs? | 22:12 |
| jroll | no, the gate is broken, it needs to be fixed asap. | 22:12 |
| jroll | you do | 22:12 |
| jroll | but... eventually | 22:12 |
| devananda | what's the borkennes? | 22:12 |
| jroll | does that make sense? | 22:12 |
| jlvillal | jroll: Also I think the job will fail because the reqs don't match. If I remember correctly. | 22:12 |
| jroll | devananda: new httpretty breaks client tests | 22:12 |
| devananda | oh nice | 22:12 |
| jroll | jlvillal: we should remove that job, it's dumb. | 22:12 |
| jlvillal | It came out today. | 22:12 |
| jlvillal | jroll: Okay with me :) I'm not sure who can do that. | 22:13 |
| jroll | everyone :) | 22:13 |
| * jroll finds it | 22:13 | |
| jroll | jlvillal: in the meantime can you propose that change to the client? | 22:13 |
| jlvillal | jroll: maybe it could be non-voting | 22:13 |
| jlvillal | jroll: Will do! | 22:14 |
| jroll | mehhh. | 22:14 |
| *** penick has quit IRC | 22:14 | |
| devananda | jroll: what job? | 22:14 |
| jroll | devananda: "does requirements.txt match global" | 22:14 |
| devananda | jroll: no, we need that. | 22:14 |
| devananda | jroll: because PIP has no dependency resolver | 22:14 |
| jroll | I thought we removed it elsewhere? | 22:15 |
| devananda | jroll: fix PIP and then we can remove that | 22:15 |
| jroll | like, we don't have that job in ironic | 22:15 |
| devananda | eh..? | 22:15 |
| *** ijw has joined #openstack-ironic | 22:15 | |
| jroll | I don't think so, anyway. | 22:15 |
| adam_g | that test is part of ironic | 22:15 |
| adam_g | er | 22:15 |
| adam_g | devstack | 22:15 |
| devananda | right | 22:15 |
| *** arif-ali has quit IRC | 22:15 | |
| jroll | oh. | 22:15 |
| devananda | we talked about moving it to a "install everything in devstack" test | 22:15 |
| devananda | adam_g: so ^ is done now? | 22:15 |
| openstackgerrit | John L. Villalovos proposed openstack/python-ironicclient: Avoid httpretty 0.8.8 as it can break unittests https://review.openstack.org/163629 | 22:16 |
| *** lucas-dinner has quit IRC | 22:16 | |
| adam_g | devananda, im not sure i know about that | 22:16 |
| devananda | adam_g: mmkay. so what do you mean by 'that test is part of.. devstack' ? | 22:17 |
| adam_g | devananda, devstack does some syncing and checking of project requirements vs global-requirements | 22:17 |
| jroll | alrighty then, ignore me on removing the job | 22:17 |
| *** ijw has joined #openstack-ironic | 22:17 | |
| jroll | pinned infra with the g-r patch | 22:17 |
| adam_g | so if a project is listing something wrong, devstack bails out with an error and fails the dsvm job | 22:18 |
| lifeless | jlvillal: done | 22:18 |
| *** arif-ali has joined #openstack-ironic | 22:18 | |
| devananda | adam_g: oh i see. | 22:19 |
| devananda | adam_g: huh. but devstack installs clients from pip | 22:19 |
| jroll | devananda: on a client change? | 22:20 |
| adam_g | devananda, not in the case of devstack jobs running against that client | 22:20 |
| devananda | adam_g: oh, right. duh | 22:20 |
| adam_g | python-ironicclient changes get tested with a checkout of that change | 22:20 |
| devananda | yep yep | 22:20 |
| devananda | adam_g: nope! http://logs.openstack.org/02/163202/1/gate/gate-tempest-dsvm-ironic-pxe_ssh/f006348/logs/devstacklog.txt.gz#_2015-03-11_18_37_24_282 | 22:22 |
| adam_g | devananda, hmm! thats a bug, good catch | 22:22 |
| jroll | devananda: wow. | 22:24 |
| jroll | so our client is completely untested, that's cool | 22:24 |
| devananda | adam_g: so, this test is pulling from git master -- http://logs.openstack.org/02/163202/1/gate/gate-tempest-dsvm-neutron-src-python-ironicclient/f936171/logs/devstack-gate-setup-workspace-new.txt.gz#_2015-03-11_18_27_57_157 | 22:25 |
| jroll | even though this ran: 2015-03-11 18:37:23.037 | ++ use_library_from_git python-ironicclient | 22:25 |
| devananda | but I dont know what that test is doing | 22:25 |
| * devananda looks at project config | 22:25 | |
| adam_g | patch inc | 22:25 |
| adam_g | https://review.openstack.org/163632 | 22:27 |
| devananda | adam_g: jenkins/jobs/devstack-gate.yaml L 167 | 22:28 |
| devananda | 171 # Purpose: this allows libraries to test their proposed commits to | 22:28 |
| devananda | 172 # ensure they don't break OpenStack on their next release. It is | 22:28 |
| devananda | so we are running it, like you thought, but the job name is different | 22:28 |
| jlvillal | lifeless: That won't work what you suggested. My other patch fixed what you are suggesting. | 22:29 |
| adam_g | devananda, yeah, we dont run it on the job we actually care about | 22:29 |
| jroll | jlvillal: why wouldn't that work? | 22:29 |
| jlvillal | lifeless: The issue is that if during the yield an exception occurrs, it gets converted into a PasswordFileFailedToCreate exception | 22:29 |
| devananda | yea | 22:29 |
| jlvillal | jroll: ^^ | 22:30 |
| jroll | hmm | 22:30 |
| jlvillal | jroll: Which is what happened before. | 22:30 |
| devananda | jroll: so actually yea, it looks like we aren't running functional tests on client changes, only unit tests :( | 22:30 |
| jroll | devananda: indeed. | 22:31 |
| lifeless | jlvillal: ahha! | 22:31 |
| lifeless | jlvillal: with you now. | 22:31 |
| lifeless | jlvillal: let me noodle a little | 22:32 |
| jlvillal | lifeless: Good :) I also replied in the patch | 22:32 |
| jlvillal | lifeless: If you look at the bug https://bugs.launchpad.net/ironic/+bug/1428722 I went into more detail. | 22:33 |
| openstack | Launchpad bug 1428722 in Ironic "Ironic drivers that utilize temp files should verify they can actually create them" [Low,In progress] - Assigned to Chris Krelle (nobodycam) | 22:33 |
| jlvillal | lifeless: Which reminds me I was supposed to say: Partially-fixed or something like that and reference that bug. | 22:34 |
| lifeless | jlvillal: its obvious now, brain-thunk on my part. | 22:34 |
| jlvillal | lifeless: I'm glad it is obvious for you. It took me a little while to figure it out :D | 22:35 |
| lifeless | *now*, not before you mentioned :) hindsight etc | 22:35 |
| lifeless | I think it deserves a comment. | 22:35 |
| jlvillal | Same thing happens to me | 22:35 |
| jlvillal | lifeless: Okay, I can do that. By the way do you know the way to say this patch partially fixes a bug? | 22:35 |
| lifeless | https://wiki.openstack.org/wiki/GitCommitMessages#Including_external_references | 22:36 |
| *** jgrimm is now known as zz_jgrimm | 22:36 | |
| lifeless | jlvillal: what do you think of https://etherpad.openstack.org/p/lifeless-scratch | 22:37 |
| lifeless | oops, tweaking | 22:37 |
| lifeless | there | 22:37 |
| jlvillal | lifeless: Sure, but is there much difference than what is there in the patch? I'm not seeing it. | 22:38 |
| lifeless | we don't need the delete_if_exists call at all, because thats what NamedTemporaryFile is for | 22:39 |
| lifeless | so we also don't need the check for path being None | 22:39 |
| lifeless | its just leaning on the underlying library more heavily | 22:40 |
| lifeless | reducing our code | 22:40 |
| jlvillal | lifeless: reading again... | 22:40 |
| jlvillal | lifeless: So what happens when the caller opens and closes the file? | 22:41 |
| lifeless | they're opening by path | 22:41 |
| lifeless | not fd or anything | 22:41 |
| lifeless | the del f at the end is just to ensure nothing is held as a reference in the event of exceptions. Oh, I can tweak that | 22:42 |
| jlvillal | lifeless: Okay. So no contention then. | 22:42 |
| jlvillal | lifeless: Should the yield/del be in a try/finally block? | 22:43 |
| openstackgerrit | John L. Villalovos proposed openstack/ironic: Update unittests and use NamedTemporaryFile https://review.openstack.org/162672 | 22:44 |
| lifeless | yes it should, which I've now done | 22:44 |
| lifeless | hmm, no thats wrong still, /me pokes more :) | 22:44 |
| lifeless | there | 22:45 |
| jlvillal | lifeless: Hmmm. I think the current patch works ;) | 22:45 |
| jlvillal | lifeless: But amenable to improvements :D | 22:45 |
| lifeless | sure, and I'm not worried either way. Its sometimes good to poke closely to see | 22:45 |
| jlvillal | lifeless: /me doesn't want to redo all the unittests again :) | 22:45 |
| lifeless | fair enough. The patch you have up can leak (all of them could AFAICT) | 22:47 |
| lifeless | putting details in the review | 22:47 |
| jlvillal | lifeless: Okay. Thanks. | 22:47 |
| openstackgerrit | Chris Krelle proposed openstack/ironic: Check temp dir is writable for ipmitool driver https://review.openstack.org/160383 | 22:50 |
| NobodyCam | not sure my test of ipmitool's power init is valid here anyone good with test have a second to look ^^^^^ | 22:50 |
| lifeless | jlvillal: comment up | 22:51 |
| lifeless | jlvillal: I'm not going to spin on this more - its in your court which shape you think looks best. I don't think there is a /lot/ of race difference between them. | 22:52 |
| jlvillal | lifeless: Thanks for all the help. | 22:54 |
| lifeless | its a shame that writing correct code is so hard in Python | 22:55 |
| lifeless | ok-code is easy, correct is hard :) | 22:55 |
| jlvillal | lifeless: :) | 22:55 |
| openstackgerrit | Chris Krelle proposed openstack/ironic: Check temp dir is writable for ipmitool driver https://review.openstack.org/160383 | 22:56 |
| *** penick has joined #openstack-ironic | 22:56 | |
| *** penick has quit IRC | 22:56 | |
| openstackgerrit | Josh Gachnang proposed openstack/ironic: Implement execute clean steps https://review.openstack.org/155561 | 23:24 |
| *** romcheg has joined #openstack-ironic | 23:27 | |
| *** romcheg has quit IRC | 23:27 | |
| *** jmccrory1 has quit IRC | 23:28 | |
| openstackgerrit | Josh Gachnang proposed openstack/ironic-python-agent: Add dispatch to all managers https://review.openstack.org/161001 | 23:32 |
| openstackgerrit | Josh Gachnang proposed openstack/ironic-python-agent: Add dispatch to all managers https://review.openstack.org/161001 | 23:32 |
| jlvillal | lifeless: I was trying the code you suggested. Seems to work. But then flake8 complains and says F821 undefined name 'f' for the 'yield f.name' :( | 23:33 |
| * jlvillal thinks a bug in flake8 | 23:34 | |
| lifeless | yeah, thats messed up :) | 23:34 |
| jlvillal | lifeless: Yeah, flake8 sees the 'del f' in the except block | 23:34 |
| mrda | #qa :) | 23:34 |
| lifeless | for f to be None, we have to have not raised in that raise | 23:35 |
| jlvillal | lifeless: If I comment that out it doesn't think it doesn't exist | 23:35 |
| * mrda is not serious | 23:35 | |
| lifeless | we can't have nice things | 23:35 |
| jlvillal | lifeless: If I comment out 'del f', then there is no error | 23:35 |
| * jlvillal argh! | 23:35 | |
| mrda | Every bug we find is an opportunity to fix something | 23:35 |
| lifeless | the optimism, it burns | 23:36 |
| jlvillal | mrda: Your # qa idea sounds like it might be the solution. For the two lines... | 23:36 |
| mrda | You might need to add a comment to say why you've #qa'd it | 23:36 |
| mrda | Perhaps raise a bug for flake8 and reference it in the comment | 23:37 |
| mrda | as a TODO | 23:37 |
| mrda | jlvillal: if you don't mind, shoot me the code review reference when you've submitted it | 23:40 |
| *** kozhukalov has quit IRC | 23:40 | |
| jlvillal | mrda: Will do. | 23:40 |
| mrda | ta | 23:41 |
| jlvillal | mrda: Currently this: https://review.openstack.org/162672 was trying to implement lifeless suggestion: https://etherpad.openstack.org/p/lifeless-scratch | 23:41 |
| *** chlong has joined #openstack-ironic | 23:41 | |
| mrda | thnx | 23:43 |
| *** absubram has quit IRC | 23:44 | |
| openstackgerrit | John L. Villalovos proposed openstack/ironic: Update unittests and use NamedTemporaryFile https://review.openstack.org/162672 | 23:48 |
| jlvillal | mrda: lifeless: ^^^^^^ | 23:48 |
| *** penick has joined #openstack-ironic | 23:48 | |
| *** Haomeng|2 has joined #openstack-ironic | 23:49 | |
| *** Haomeng has quit IRC | 23:51 | |
| *** mtanino has joined #openstack-ironic | 23:52 | |
| lifeless | jlvillal: you need both # noqa's ? | 23:52 |
| jlvillal | lifeless: I think so.... | 23:52 |
| * jlvillal goes to check | 23:52 | |
| *** rloo has quit IRC | 23:53 | |
| jlvillal | lifeless: Yep. If I delete the 2nd one I get an error: ./ironic/drivers/modules/ipmitool.py:208:13: F821 undefined name 'f' | 23:53 |
| lifeless | ahahahaha | 23:54 |
| mrda | thanks jlvillal | 23:54 |
| lifeless | so broken | 23:54 |
| *** penick has quit IRC | 23:59 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!