Thursday, 2017-05-18

openstackgerritIan Wienand proposed openstack/diskimage-builder master: Set manifests to mode 600 and owner root  https://review.openstack.org/46565500:11
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Remove _config_error thrower  https://review.openstack.org/46363000:17
openstackgerritMerged openstack/diskimage-builder master: Remove PluginBase/NodePluginBase class  https://review.openstack.org/46453800:24
*** jamielennox is now known as jamielennox|away00:27
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Remove _config_error thrower  https://review.openstack.org/46363000:38
*** pmannidi has quit IRC00:41
*** jamielennox|away is now known as jamielennox00:41
*** Sukhdev has joined #openstack-dib00:43
*** pmannidi has joined #openstack-dib01:03
*** Sukhdev has quit IRC01:06
openstackgerritMerged openstack/diskimage-builder master: Remove _config_error thrower  https://review.openstack.org/46363002:37
*** jamielennox is now known as jamielennox|away02:40
*** jamielennox|away is now known as jamielennox03:33
*** Sukhdev has joined #openstack-dib04:13
*** chhavi has joined #openstack-dib04:21
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Start at cleaning up tree/graph config split  https://review.openstack.org/46541704:30
*** andreas-f has joined #openstack-dib04:37
*** Sukhdev has quit IRC05:28
*** aparnav has joined #openstack-dib06:39
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Split partition into it's own file  https://review.openstack.org/46583706:48
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Move parts of Partition creation into object  https://review.openstack.org/46583806:49
yolandahi, good morning06:49
yolandaso ianw, how are things? are there some changes to review?06:49
yolandai 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 concepts06:50
ianwyolanda: 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 stuff06:51
yolandayes, i prefer that, less complex and easier to understand06:55
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Start at cleaning up tree/graph config split  https://review.openstack.org/46541706:55
ianw465417 is part 1 ...06:55
ianwthen, 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/46541706:57
ianwin that way, we don't have to special case-them06:57
ianwthen, 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 creation06:57
ianwcreate() on a "partition" object does nothing ... it's just a place-holder in the graph06:57
ianwcurrently, create() on a partition object calls back into the parent object, which has a global flag "have we done this or not"06:58
yolandaone 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 partitions07:00
ianwyolanda: 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 plugins07:03
yolandayes, all trees can be graphs07:03
ianwotherwise, plugins can essentially start making up their own config structures07:03
ianwwhich is a) difficult to maintain and b) going to confuse the heck out of maintainers07:04
ianwsorry, users ...07:04
ianwalready, "partitioning" has a "special" entry "partitions" which is a list, which is parsed via the duplicated code in partitioning.py07:05
ianwto be concrete, the config file -> https://review.openstack.org/#/c/465417/6/diskimage_builder/block_device/tests/config/multiple_partitions_tree.yaml07:06
ianwis turned into -> https://review.openstack.org/#/c/465417/6/diskimage_builder/block_device/tests/config/multiple_partitions_graph.yaml07:07
yolandai understand the idea and yes, i think it works. Not all graphs can be trees, but all trees are graphs...07:08
ianwright07:08
ianwbut i definitely think plugins should know *nothing* about trees/graphs etc.  they get the config in a canonical graph form, and that's that07:10
ianwand i'd also like to have as much test-driven development here as practical07:10
ianwthat probably means a lot more use of mocking to unit test some bits and pieces07:11
ianwi 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
ianwthe answer to that is that the calls should be mocked...07:11
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Move parts of Partition creation into object  https://review.openstack.org/46583807:18
yolandaso you mean, in functional testing we could mock calls?07:21
ianwyolanda: in a lot of the unit testing07:22
ianwlike in http://git.openstack.org/cgit/openstack/diskimage-builder/tree/diskimage_builder/tests/functional/test_blockdevice.py#n6107:24
ianwplaces where we do externals, we need to mock out the calls and return dummy value07:24
ianwanyway, it's a lot of work, don't expect it to happen immediately07:24
ianwyolanda / 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 done07:25
ianwthen there is *no* special handling of "partitions" and all the TreeConfig() and special handling goes away...07:25
yolandanot sure if i follow that, i find this part of the code quite difficult to understand07:28
ianwthe specific thing that would make this work is line 54 of https://review.openstack.org/#/c/465417/6/diskimage_builder/block_device/config.py07:29
ianwif we see a list, we just process each element in the list one-by-one07:29
ianwthis means "partition:" becomes a list of partition objects, and is not special-cased as it is now07:30
ianwyolanda: yeah ... that's why i'm trying to present the changes in smaller chunks07:30
ianwi'm not quite there :)07:31
ianwhttps://review.openstack.org/#/c/465837/ should be pretty clear07:31
yolandai understand your general idea, but is difficult to follow the details07:32
yolandaso 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 way07:33
ianwyolanda: yep ... if you get your head around what happens with the "partitions:" section right now, that's probably the key to it07:33
ianwwe'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#n12607:35
ianwwe end up on a separate path building building up this "partitions" key for the config07:35
ianwi just want a list of "partition" objects to associate themselves with a "partitioning" object07: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, etc07:37
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Move parts of Partition creation into object  https://review.openstack.org/46583807:38
ianwthat'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
ianwsubtle but important difference!07:39
yolandaah, i didn't notice it!07:40
yolandamakes 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, lvs07:41
ianwyolanda: 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 do07:45
ianwyolanda: 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 it07:48
ianwit walks its children and finds them, collates them, makes the calls it needs to07:48
ianwyolanda: btw, any thoughts on https://review.openstack.org/#/c/465655/ (the manifest perms thing?)  i'm also happy to remove it, if we want07:50
yolandayes, no need to create a vlm class inside the tree or graph07:52
yolandaianw, i really have no idea of the usage of DIB_MANIFEST_SAVE_DIR. I just see it being created, but not being consumed07:54
ianwyeah, i wonder if it's more trouble than it's worth ...07:54
yolandamaybe greghaynes will know more. We can approve that patch, and revisit later if we can remove it07:56
ianwthat was what i figured ... it seems to address the immediate issue07:58
yolandaapproved07:59
yolandaianw so is there some change you consider closed, that i can start reviewing? or you need some help or feedback on any?08:06
ianwi'm hoping 465838 passes CI08:07
ianw465837 is a no-op, just for clarity08:07
ianwthen i'm a bit more stuck on getting 465417 split up08:09
ianwthe comment in https://review.openstack.org/#/c/465838/2/diskimage_builder/block_device/level1/partitioning.py is where i want to take it08:10
ianwi want the partitioning object to find the sub-partitions by looking at the graph and finding the "partition" nodes attached to it08:10
ianwi'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
yolandaok let's see if i find some time08:13
yolandadealing with tripleo in parallel so i may have time between deploy and deploy :)08:14
ianwno worries.08:14
ianwmaybe 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
yolandaso yes, order is important in that case, because of the creation of extended and logical partition08:16
yolandaso you first need to create the extended partition, then add the logicals after it08:16
ianwthat makes sense08:17
ianwbut that should be encoded within the graph?  I mean the nodes should point to each other in order?08:18
ianwthe partition nodes08:18
yolandaat the moment, we were trusting in the order, yes08:20
openstackgerritMerged openstack/diskimage-builder master: Set manifests to mode 600 and owner root  https://review.openstack.org/46565508:37
*** chhavi has quit IRC08:40
*** chhavi has joined #openstack-dib08:40
*** pmannidi has quit IRC09:45
*** isaacb has joined #openstack-dib09:54
*** jamielennox is now known as jamielennox|away10:04
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Start at cleaning up tree/graph config split  https://review.openstack.org/46541710:05
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Switch to using "partition" objects  https://review.openstack.org/46591610:30
ianwyolanda / andreas-f: ^ that's sort of the minimal, ultimate goal ...10:31
ianwi may be misunderstanding the graph generation, but it seems like if partitions have dependencies, the should be chained like "mbr" -> partition1 -> partition2 etc10:33
yolandai'll take a look this afternoon on all it10:34
yolandaalso for the security images, we may go with just partitions at this release, because it's more solid, then move to volumes on next one10:35
*** openstackgerrit has quit IRC10:48
*** aparnav has quit IRC11:44
*** openstackgerrit has joined #openstack-dib11:49
openstackgerritNoam Angel proposed openstack/diskimage-builder master: dhcp-all-interfaces.sh - Add support for InfiniBand interface DHCP  https://review.openstack.org/46592811:49
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Switch to using "partition" objects  https://review.openstack.org/46591611:52
*** jamielennox|away is now known as jamielennox13:49
*** isaacb has quit IRC16:48
openstackgerritJesse Keating proposed openstack/diskimage-builder master: Remove use of 'which'.  https://review.openstack.org/46606318:09
*** chhavi has quit IRC18:24
*** Sukhdev has joined #openstack-dib18:30
*** sauloaislan has joined #openstack-dib19:32
sauloaislanhi everybody!!19:32
sauloaislanPlease what is the command to create a qcow2 image with sshkey or username and password?19:33
*** Sukhdev has quit IRC19:35
openstackgerritJesse Keating proposed openstack/diskimage-builder master: Remove use of 'which'.  https://review.openstack.org/46606319:39
*** Sukhdev has joined #openstack-dib20:49
openstackgerritJesse Keating proposed openstack/diskimage-builder master: Remove use of 'which'.  https://review.openstack.org/46606320:56
openstackgerritJesse Keating proposed openstack/diskimage-builder master: Remove use of 'which'.  https://review.openstack.org/46606322:13
*** Sukhdev has quit IRC22:20
*** Sukhdev has joined #openstack-dib22:36
*** pmannidi has joined #openstack-dib23:11
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Switch to using "partition" objects  https://review.openstack.org/46591623:52
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Move parts of Partition creation into object  https://review.openstack.org/46583823:52
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Split partition into it's own file  https://review.openstack.org/46583723:52
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Start at cleaning up tree/graph config split  https://review.openstack.org/46541723:52
openstackgerritIan Wienand proposed openstack/diskimage-builder master: Move exception to it's own file (again)  https://review.openstack.org/46612923:52

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