openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Set manifests to mode 600 and owner root https://review.openstack.org/465655 | 00:11 |
---|---|---|
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Remove _config_error thrower https://review.openstack.org/463630 | 00:17 |
openstackgerrit | Merged openstack/diskimage-builder master: Remove PluginBase/NodePluginBase class https://review.openstack.org/464538 | 00:24 |
*** jamielennox is now known as jamielennox|away | 00:27 | |
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Remove _config_error thrower https://review.openstack.org/463630 | 00:38 |
*** pmannidi has quit IRC | 00:41 | |
*** jamielennox|away is now known as jamielennox | 00:41 | |
*** Sukhdev has joined #openstack-dib | 00:43 | |
*** pmannidi has joined #openstack-dib | 01:03 | |
*** Sukhdev has quit IRC | 01:06 | |
openstackgerrit | Merged openstack/diskimage-builder master: Remove _config_error thrower https://review.openstack.org/463630 | 02:37 |
*** jamielennox is now known as jamielennox|away | 02:40 | |
*** jamielennox|away is now known as jamielennox | 03:33 | |
*** Sukhdev has joined #openstack-dib | 04:13 | |
*** chhavi has joined #openstack-dib | 04:21 | |
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Start at cleaning up tree/graph config split https://review.openstack.org/465417 | 04:30 |
*** andreas-f has joined #openstack-dib | 04:37 | |
*** Sukhdev has quit IRC | 05:28 | |
*** aparnav has joined #openstack-dib | 06:39 | |
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Split partition into it's own file https://review.openstack.org/465837 | 06:48 |
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Move parts of Partition creation into object https://review.openstack.org/465838 | 06:49 |
yolanda | hi, good morning | 06:49 |
yolanda | so ianw, how are things? are there some changes to review? | 06:49 |
yolanda | i could land some changes yesterday, and i need to continue working on the volumes change, but i think that it's better that i resume it after your refactor, as it's changing base concepts | 06:50 |
ianw | yolanda: yeah, i'm trying not to actually change too much ... but i think we can greatly simplify the partition stuff and get rid of the tree_to_diagraph passthrough stuff | 06:51 |
yolanda | yes, i prefer that, less complex and easier to understand | 06:55 |
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Start at cleaning up tree/graph config split https://review.openstack.org/465417 | 06:55 |
ianw | 465417 is part 1 ... | 06:55 |
ianw | then, i think we modify the special handling of "partitions" to just be a list of "partition" s ... this is better seen in https://review.openstack.org/465417 | 06:57 |
ianw | in that way, we don't have to special case-them | 06:57 |
ianw | then, when we call create() on the partition object, it *should* be able to look at the graph, and figure out what partitions to include in the creation | 06:57 |
ianw | create() on a "partition" object does nothing ... it's just a place-holder in the graph | 06:57 |
ianw | currently, create() on a partition object calls back into the parent object, which has a global flag "have we done this or not" | 06:58 |
yolanda | one thing that you need to keep in mind, is that partitions still follow a tree structure even if we do it on a graph. But for volumes, it may not be the same, it will be a real graph. For the case when you can create a volume group, that references different physical volumes on different partitions | 07:00 |
ianw | yolanda: but any tree is representable as the graph, right? that's why i'd like there to be *one* call that converts the tree to a graph. and that should seem to be independent of all plugins | 07:03 |
yolanda | yes, all trees can be graphs | 07:03 |
ianw | otherwise, plugins can essentially start making up their own config structures | 07:03 |
ianw | which is a) difficult to maintain and b) going to confuse the heck out of maintainers | 07:04 |
ianw | sorry, users ... | 07:04 |
ianw | already, "partitioning" has a "special" entry "partitions" which is a list, which is parsed via the duplicated code in partitioning.py | 07:05 |
ianw | to be concrete, the config file -> https://review.openstack.org/#/c/465417/6/diskimage_builder/block_device/tests/config/multiple_partitions_tree.yaml | 07:06 |
ianw | is turned into -> https://review.openstack.org/#/c/465417/6/diskimage_builder/block_device/tests/config/multiple_partitions_graph.yaml | 07:07 |
yolanda | i understand the idea and yes, i think it works. Not all graphs can be trees, but all trees are graphs... | 07:08 |
ianw | right | 07:08 |
ianw | but i definitely think plugins should know *nothing* about trees/graphs etc. they get the config in a canonical graph form, and that's that | 07:10 |
ianw | and i'd also like to have as much test-driven development here as practical | 07:10 |
ianw | that probably means a lot more use of mocking to unit test some bits and pieces | 07:11 |
ianw | i noticed in the tests there's comments like "this needs sudo which is not allowed, i don't know what to do here" | 07:11 |
ianw | the answer to that is that the calls should be mocked... | 07:11 |
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Move parts of Partition creation into object https://review.openstack.org/465838 | 07:18 |
yolanda | so you mean, in functional testing we could mock calls? | 07:21 |
ianw | yolanda: in a lot of the unit testing | 07:22 |
ianw | like in http://git.openstack.org/cgit/openstack/diskimage-builder/tree/diskimage_builder/tests/functional/test_blockdevice.py#n61 | 07:24 |
ianw | places where we do externals, we need to mock out the calls and return dummy value | 07:24 |
ianw | anyway, it's a lot of work, don't expect it to happen immediately | 07:24 |
ianw | yolanda / andreas-f : see my comment in https://review.openstack.org/#/c/465838/2/diskimage_builder/block_device/level1/partitioning.py about the partitions. that is what i would like to see done | 07:25 |
ianw | then there is *no* special handling of "partitions" and all the TreeConfig() and special handling goes away... | 07:25 |
yolanda | not sure if i follow that, i find this part of the code quite difficult to understand | 07:28 |
ianw | the specific thing that would make this work is line 54 of https://review.openstack.org/#/c/465417/6/diskimage_builder/block_device/config.py | 07:29 |
ianw | if we see a list, we just process each element in the list one-by-one | 07:29 |
ianw | this means "partition:" becomes a list of partition objects, and is not special-cased as it is now | 07:30 |
ianw | yolanda: yeah ... that's why i'm trying to present the changes in smaller chunks | 07:30 |
ianw | i'm not quite there :) | 07:31 |
ianw | https://review.openstack.org/#/c/465837/ should be pretty clear | 07:31 |
yolanda | i understand your general idea, but is difficult to follow the details | 07:32 |
yolanda | so we can pass any graph with a list of items, just with the right type, name and base, and code shall process in a generic way | 07:33 |
ianw | yolanda: yep ... if you get your head around what happens with the "partitions:" section right now, that's probably the key to it | 07:33 |
ianw | we're walking the config tree, and if we see "partitions" in http://git.openstack.org/cgit/openstack/diskimage-builder/tree/diskimage_builder/block_device/level1/partitioning.py#n126 | 07:35 |
ianw | we end up on a separate path building building up this "partitions" key for the config | 07:35 |
ianw | i just want a list of "partition" objects to associate themselves with a "partitioning" object | 07:37 |
ianw | "partitioning" could probably be more accurately be called "boot_record" or something ... i guess in theory it could be a mbr, gpt, whatever ppc uses, etc | 07:37 |
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Move parts of Partition creation into object https://review.openstack.org/465838 | 07:38 |
ianw | that's why, in https://review.openstack.org/#/c/465417/6/diskimage_builder/block_device/tests/config/multiple_partitions_tree.yaml , note that it's *not* "partitions" (the special key) ... it's a list of "partition" | 07:39 |
ianw | subtle but important difference! | 07:39 |
yolanda | ah, i didn't notice it! | 07:40 |
yolanda | makes sense, i was also trying to reproduce that schema in volumes, adding an "lvm" class. That could really be dropped, and just have a list of pvs, vgs, lvs | 07:41 |
ianw | yolanda: i think that's right ... i think if we get this partitioning idea right, the rest should fall into place ... especially as per 465838 if the "parent" node can just collate the sub-nodes and do what it needs to do | 07:45 |
ianw | yolanda: i think you'd still have the lvm "type"/plugin whatever we want to call it. but it would just have "pvs", "vgs", "lvs" children under it | 07:48 |
ianw | it walks its children and finds them, collates them, makes the calls it needs to | 07:48 |
ianw | yolanda: btw, any thoughts on https://review.openstack.org/#/c/465655/ (the manifest perms thing?) i'm also happy to remove it, if we want | 07:50 |
yolanda | yes, no need to create a vlm class inside the tree or graph | 07:52 |
yolanda | ianw, i really have no idea of the usage of DIB_MANIFEST_SAVE_DIR. I just see it being created, but not being consumed | 07:54 |
ianw | yeah, i wonder if it's more trouble than it's worth ... | 07:54 |
yolanda | maybe greghaynes will know more. We can approve that patch, and revisit later if we can remove it | 07:56 |
ianw | that was what i figured ... it seems to address the immediate issue | 07:58 |
yolanda | approved | 07:59 |
yolanda | ianw so is there some change you consider closed, that i can start reviewing? or you need some help or feedback on any? | 08:06 |
ianw | i'm hoping 465838 passes CI | 08:07 |
ianw | 465837 is a no-op, just for clarity | 08:07 |
ianw | then i'm a bit more stuck on getting 465417 split up | 08:09 |
ianw | the comment in https://review.openstack.org/#/c/465838/2/diskimage_builder/block_device/level1/partitioning.py is where i want to take it | 08:10 |
ianw | i want the partitioning object to find the sub-partitions by looking at the graph and finding the "partition" nodes attached to it | 08:10 |
ianw | i'm not going to get to hacking on that tonight, but maybe you want to try looking at 465417 and seeing if that's possible, or at least getting more familiar with the code for more review :) | 08:13 |
yolanda | ok let's see if i find some time | 08:13 |
yolanda | dealing with tripleo in parallel so i may have time between deploy and deploy :) | 08:14 |
ianw | no worries. | 08:14 |
ianw | maybe look at prev_partition ... i'm *still* not clear what role that is playing in the whole thing? enforcing ordering? does ordering matter? if nothing else comments would help :) | 08:15 |
yolanda | so yes, order is important in that case, because of the creation of extended and logical partition | 08:16 |
yolanda | so you first need to create the extended partition, then add the logicals after it | 08:16 |
ianw | that makes sense | 08:17 |
ianw | but that should be encoded within the graph? I mean the nodes should point to each other in order? | 08:18 |
ianw | the partition nodes | 08:18 |
yolanda | at the moment, we were trusting in the order, yes | 08:20 |
openstackgerrit | Merged openstack/diskimage-builder master: Set manifests to mode 600 and owner root https://review.openstack.org/465655 | 08:37 |
*** chhavi has quit IRC | 08:40 | |
*** chhavi has joined #openstack-dib | 08:40 | |
*** pmannidi has quit IRC | 09:45 | |
*** isaacb has joined #openstack-dib | 09:54 | |
*** jamielennox is now known as jamielennox|away | 10:04 | |
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Start at cleaning up tree/graph config split https://review.openstack.org/465417 | 10:05 |
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Switch to using "partition" objects https://review.openstack.org/465916 | 10:30 |
ianw | yolanda / andreas-f: ^ that's sort of the minimal, ultimate goal ... | 10:31 |
ianw | i may be misunderstanding the graph generation, but it seems like if partitions have dependencies, the should be chained like "mbr" -> partition1 -> partition2 etc | 10:33 |
yolanda | i'll take a look this afternoon on all it | 10:34 |
yolanda | also for the security images, we may go with just partitions at this release, because it's more solid, then move to volumes on next one | 10:35 |
*** openstackgerrit has quit IRC | 10:48 | |
*** aparnav has quit IRC | 11:44 | |
*** openstackgerrit has joined #openstack-dib | 11:49 | |
openstackgerrit | Noam Angel proposed openstack/diskimage-builder master: dhcp-all-interfaces.sh - Add support for InfiniBand interface DHCP https://review.openstack.org/465928 | 11:49 |
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Switch to using "partition" objects https://review.openstack.org/465916 | 11:52 |
*** jamielennox|away is now known as jamielennox | 13:49 | |
*** isaacb has quit IRC | 16:48 | |
openstackgerrit | Jesse Keating proposed openstack/diskimage-builder master: Remove use of 'which'. https://review.openstack.org/466063 | 18:09 |
*** chhavi has quit IRC | 18:24 | |
*** Sukhdev has joined #openstack-dib | 18:30 | |
*** sauloaislan has joined #openstack-dib | 19:32 | |
sauloaislan | hi everybody!! | 19:32 |
sauloaislan | Please what is the command to create a qcow2 image with sshkey or username and password? | 19:33 |
*** Sukhdev has quit IRC | 19:35 | |
openstackgerrit | Jesse Keating proposed openstack/diskimage-builder master: Remove use of 'which'. https://review.openstack.org/466063 | 19:39 |
*** Sukhdev has joined #openstack-dib | 20:49 | |
openstackgerrit | Jesse Keating proposed openstack/diskimage-builder master: Remove use of 'which'. https://review.openstack.org/466063 | 20:56 |
openstackgerrit | Jesse Keating proposed openstack/diskimage-builder master: Remove use of 'which'. https://review.openstack.org/466063 | 22:13 |
*** Sukhdev has quit IRC | 22:20 | |
*** Sukhdev has joined #openstack-dib | 22:36 | |
*** pmannidi has joined #openstack-dib | 23:11 | |
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Switch to using "partition" objects https://review.openstack.org/465916 | 23:52 |
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Move parts of Partition creation into object https://review.openstack.org/465838 | 23:52 |
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Split partition into it's own file https://review.openstack.org/465837 | 23:52 |
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Start at cleaning up tree/graph config split https://review.openstack.org/465417 | 23:52 |
openstackgerrit | Ian Wienand proposed openstack/diskimage-builder master: Move exception to it's own file (again) https://review.openstack.org/466129 | 23:52 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!