[vdsm] clean testable code
Talking with my teammates I've noticed that there is some confusion related to what testing means and in general how tests can improve the quality of the codebase by favouring the development of components that are of higher quality - unluckily it doesn't favour hair regrowing. Tests are also cool because when done right they represent an executable form of documentation. Some intro material: 1 How to Write Clean, Testable Code by Miško Hevery (A Googler) http://www.youtube.com/watch?v=XcT4yYu_TTs 2 How to write testable code by Miško Hevery (hands on ;-)) http://misko.hevery.com/attachments/Guide-Writing%20Testable%20Code.pdf Examples are provided in Java but the same principles apply to Python as well. 3 TDD by James Shore (a practice created by Kent Beck) http://www.jamesshore.com/Agile-Book/test_driven_development.html A broad article explaining TDD and related concepts. 4 Difference between state based testing and interaction based testing http://martinfowler.com/articles/mocksArentStubs.html Not so intro - but easier to understand after consuming previous material: 5 Mock Roles, not Objects by Freeman and Pryce http://jmock.org/oopsla2004.pdf They both wrote a very interesting book: Growing Object Oriented Software: Guided by Tests http://www.amazon.com/Growing-Object-Oriented-Software-Guided-Tests/dp/0321503627/ref=cm_cr_pr_product_top I didn't read it yet. Points from 3 to 5 mostly describe a different kind of working practice, which is very opinionated but nevertheless with some interesting ideas. I consider points 1-2 fundamentals. Enjoy, Giuseppe I will share new materials if there's some interest on it. ___ vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
Re: [vdsm] broken test FileVolumeGetVSizeTest in git master
- Original Message - | From: "Antoni Segura Puimedon" | To: "VDSM Project Development" | Sent: Monday, August 5, 2013 7:06:00 PM | Subject: [vdsm] broken test FileVolumeGetVSizeTest in git master | | Hi oVirters, | | Apparently due to commit: | | commit a3b4b2cb6ee0ea8dbe04413882d52d8a0e776ef8 | Author: Eduardo Warszawski | Date: Wed Jul 31 04:49:52 2013 +0300 | | Avoid Img and Vol produces in fileVolume.getV*Size | | No need for produce Images and Volumes for get the volume path | when calculating the size. | | Related to BZ#960952, BZ#769502. | | which adds the line: | | volPath = os.path.join(sdobj.mountpoint, sdobj.sdUUID, 'images', | | the test in $subj, FileVolumeGetVSizeTest is broken, since it calls: | | volSize = FileVolume.getVSize(self.sdobj, self.imgUUID, self.volUUID) | | that uses FileDomainMockObject (which does not have a mountpoint), resulting | in | |AttributeError: 'FileDomainMockObject' object has no attribute |'mountpoint' | | Best regards, | | Toni I've submitted a patch for it http://gerrit.ovirt.org/#/c/17705/1 Giuseppe | ___ | vdsm-devel mailing list | vdsm-devel@lists.fedorahosted.org | https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel | ___ vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
Re: [vdsm] delNetwork issue
- Original Message -
| From: "Sandro Bonazzola"
| To: vdsm-devel@lists.fedorahosted.org, "Giuseppe Vallarelli"
, "Antoni Segura Puimedon"
|
| Sent: Friday, June 7, 2013 10:05:12 AM
| Subject: delNetwork issue
|
| After having created the engine bridge, trying to remove it leads to:
|
|
| # /usr/share/vdsm/delNetwork engine '' '' em1
| INFO:root:Removing network engine with vlan=None, bonding=None,
| nics=['em1'],options={}
| Traceback (most recent call last):
| File "/usr/share/vdsm/configNetwork.py", line 666, in
| main()
| File "/usr/share/vdsm/configNetwork.py", line 641, in main
| delNetwork(bridge, **kwargs)
| File "/usr/share/vdsm/configNetwork.py", line 355, in delNetwork
| _removeUnusedNics(network, vlan, bonding, nics, configWriter)
| File "/usr/share/vdsm/configNetwork.py", line 259, in _removeUnusedNics
| ifup(nic)
| File "/usr/share/vdsm/netconf/ifcfg.py", line 739, in ifup
| rc, out, err = _ifup(iface)
| File "/usr/share/vdsm/netconf/ifcfg.py", line 728, in _ifup
| out[-1] if out else '')
| ConfigNetworkError: (29, '')
Hi Sandro, I spent some time looking into this issue, perhaps the problem relies
in netconf/ifcfg.py in removeNic, there may be an issue when resetting a nic
config but that needs verification.
Cheers, Giuseppe
|
| --
| Sandro Bonazzola
| Better technology. Faster innovation. Powered by community collaboration.
| See how it works at redhat.com
|
|
___
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
Re: [vdsm] configNetwork issue
- Original Message -
| From: "Sandro Bonazzola"
| To: "Alon Bar-Lev"
| Cc: vdsm-devel@lists.fedorahosted.org
| Sent: Thursday, June 6, 2013 4:38:41 PM
| Subject: Re: [vdsm] configNetwork issue
|
| Il 06/06/2013 16:29, Alon Bar-Lev ha scritto:
| > /usr/share/vdsm/addNetwork', 'ovirtmgmt', '', '', u'eth0',
| > 'BOOTPROTO=dhcp', 'ONBOOT=yes', 'blockingdhcp=true']
|
| # /usr/share/vdsm/addNetwork ovirtmgmt '' '' em1 BOOTPROTO=dhcp
| ONBOOT=yes blockingdhcp=true
| WARNING:root:options BOOTPROTO is deprecated. Use bootproto instead
| WARNING:root:options ONBOOT is deprecated. Use onboot instead
| INFO:root:Adding network ovirtmgmt with vlan=, bonding=, nics=['em1'],
| bondingOptions=None, mtu=None, bridged=True, options={'bootproto':
| 'dhcp', 'blockingdhcp': 'true', 'onboot': 'yes'}
| Traceback (most recent call last):
| File "/usr/lib64/python2.7/runpy.py", line 162, in _run_module_as_main
| "__main__", fname, loader, pkg_name)
| File "/usr/lib64/python2.7/runpy.py", line 72, in _run_code
| exec code in run_globals
| File "/usr/share/vdsm/configNetwork.py", line 664, in
| main()
| File "/usr/share/vdsm/configNetwork.py", line 633, in main
| addNetwork(bridge, **kwargs)
| File "/usr/share/vdsm/configNetwork.py", line 198, in addNetwork
| configurator, **options)
| TypeError: objectivizeNetwork() got multiple values for keyword argument
| 'bootproto'
Hi Sandro, a very dirty and quick fix may be the following one.
Inside configNetwork reach the addNetwork function and
before the call to objectivizeNetwork:
bootproto = options.get('bootproto')
if bootproto:
del options['bootproto']
netEnt = objectivizeNetwork(network if bridged else None, vlan, bonding,
bondingOptions, nics, mtu, ipaddr, netmask,
gateway, bootproto, _netinfo,
configurator, **options)
Giuseppe
|
| --
| Sandro Bonazzola
| Better technology. Faster innovation. Powered by community collaboration.
| See how it works at redhat.com
|
| ___
| vdsm-devel mailing list
| vdsm-devel@lists.fedorahosted.org
| https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
|
___
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
Re: [vdsm] Migration regression on master
- Original Message - | From: "Assaf Muller" | To: "Dan Kenigsberg" | Cc: "Peter V. Saveliev" , vdsm-devel@lists.fedorahosted.org, "Giuseppe Vallarelli" | , "michal skrivanek" | Sent: Thursday, May 30, 2013 9:00:49 AM | Subject: Re: [vdsm] Migration regression on master | | | | * We still need to see the vmCreate command used to learn more about the | issue | * We can always implement a hack that checks if the alias attribute | exists. However, there is probably a deeper issue at hand and this is a | good opportunity to find and solve it | * Dan has a theory that the issue is with migrating a VM from a 3.0 VDSM | version host to a 3.2 host. Perhaps the balloon device isn't created | properly at the source host I've been able to reproduce the failure here there are more details: On the source host * vmCreate http://pastebin.com/yEZ3zkyN * vm xml http://pastebin.com/YSy4TYca On the dest host * vmMigrationCreate http://pastebin.com/rwdPC8Ef * _srcDomXml nicely formatted http://pastebin.com/pjE8sMMW Giuseppe | | From: "Dan Kenigsberg" | To: "Peter V. Saveliev" | Cc: vdsm-devel@lists.fedorahosted.org | Sent: Wednesday, May 29, 2013 4:41:24 PM | Subject: Re: [vdsm] Migration regression on master | | On Wed, May 29, 2013 at 02:13:02PM +0200, Peter V. Saveliev wrote: | > On 05/29/2013 07:17 AM, Dan Kenigsberg wrote: | > >On Tue, May 28, 2013 at 11:54:45AM -0400, Giuseppe Vallarelli wrote: | > >> | - Original Message - | | > >>| | From: "Assaf Muller" | > >>| | To: "Michal Skrivanek" | > >>| | Cc: "vdsm-devel@lists.fedorahosted.org Development" | > >>| | | > >>| | Sent: Thursday, May 23, 2013 2:12:47 PM | > >>| | Subject: Re: [vdsm] Migration regression on master | > >>| | | > >>| | As you can see in a previous patch set I checked if the alias | > >>| | attribute | > >>| | exists instead of assuming it exists. | > >>| | I then changed my mind with Dan's blessing, and decided to assume it | > >>| | does | > >>| | exist, exactly for cases like this. | > >>| | Even if we check if the alias exists, what do we do if we find out it | > >>| | doesn't? We're at a problem and need to understand why the alias | > >>| | doesn't | > >>| | exist because it should - For all devices. | > >>| | | > >>| | We definitely need to deal with this issue - Can you provide the | > >>| | domxml of | > >>| | the VM during creation, and during migration? | > >> | > >>Hi Assaf, Peter today has reproduced the same error and provided me | > >>the output log, you can find it here: | > >>http://etherpad.ovirt.org/p/migration-errors | > > | > >Would you add the original vmCreate line, and domxml, that was used to | > >create the VM at the source host? | > | > http://pastebin.test.redhat.com/17 | > http://pastebin.test.redhat.com/16 | | That's an internal pastebin instance. Could you simply inline the | answer? Hyperlinks makes a casual reader of the mailing list not | understand what's going on. | | Note that we need vmCreate command, the response is less | interesting, thow seeing the whole vdsm thread would not harm. | | Dan. | ___ | vdsm-devel mailing list | vdsm-devel@lists.fedorahosted.org | https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel | | ___ vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
Re: [vdsm] Migration regression on master
| - Original Message - | | From: "Assaf Muller" | | To: "Michal Skrivanek" | | Cc: "vdsm-devel@lists.fedorahosted.org Development" | | | | Sent: Thursday, May 23, 2013 2:12:47 PM | | Subject: Re: [vdsm] Migration regression on master | | | | As you can see in a previous patch set I checked if the alias attribute | | exists instead of assuming it exists. | | I then changed my mind with Dan's blessing, and decided to assume it does | | exist, exactly for cases like this. | | Even if we check if the alias exists, what do we do if we find out it | | doesn't? We're at a problem and need to understand why the alias doesn't | | exist because it should - For all devices. | | | | We definitely need to deal with this issue - Can you provide the domxml of | | the VM during creation, and during migration? Hi Assaf, Peter today has reproduced the same error and provided me the output log, you can find it here: http://etherpad.ovirt.org/p/migration-errors a previous patch blocked by this issue has been accepted (http://gerrit.ovirt.org/#/c/14203/), I've been able to do a migration using the engine. Feel free to ask if you need more details, I will look into this issue in the next few days. Giuseppe | | | | | - Original Message - | | From: "Michal Skrivanek" | | To: "Vinzenz Feenstra" , "Assaf Muller" | | | | Cc: "vdsm-devel@lists.fedorahosted.org Development" | | , "Dan Kenigsberg" | | Sent: Wednesday, May 22, 2013 3:56:22 PM | | Subject: Re: [vdsm] Migration regression on master | | | | | | On May 22, 2013, at 10:14 , Vinzenz Feenstra wrote: | | | | > Hi, | | > | | > During the verification of the refactoring in VDSM (libvirtvm + vm == vm) | | > we have encountered some issue. | | > There seems to be a regression on master branch regarding device aliases. | | > | | > On the target machine _updateDevicesDomxmlCache accesses dev.alias | | > without | | > the attribute being present. | | > | | > This happens to happen at least on balloon devices. | | > I am not sure why we do have this, however it is reproducible and I am | | > not | | > sure why we do this now. | | > | | > I know it has been introduced by http://gerrit.ovirt.org/#/c/13876 and | | > it's | | > actually in the commit message that we'd fail. | | :-) you mean: | | * We now crash horribly if an alias was not found for a device | | when updating devices domxml cache after migrating | | | | > Which is the case... So can somebody please enlighten me and tell me if | | > this is a configuration issue of the VM or if this really a regression. | | not all devices have alias so I think this is indeed a regression | | | | | | > It's possible that the tests aren't properly configured, however that | | > would | | > from my understanding mean that we potentially will run into troubles | | > during migrations from 3.2 -> 3.3 vdsm. | | > | | > PS: | | > This kind of blocks a bit the refactoring verification :/ | | > | | > -- | | > Regards, | | > | | > Vinzenz Feenstra | Senior Software Engineer | | > RedHat Engineering Virtualization R & D | | > Phone: +420 532 294 625 | | > IRC: vfeenstr or evilissimo | | > | | > Better technology. Faster innovation. Powered by community collaboration. | | > See how it works at redhat.com | | > | | > ___ | | > vdsm-devel mailing list | | > vdsm-devel@lists.fedorahosted.org | | > https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel | | | | ___ | | vdsm-devel mailing list | | vdsm-devel@lists.fedorahosted.org | | https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel | | | ___ vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
Re: [vdsm] sphinx doc generation
I've been able to generate doc for all the modules (see http://gerrit.ovirt.org/#/c/11652/) I had to change an import statement in vm.py (from: "from vdsm import vdscli" to "import vdscli") - to solve an import error generated by sphinx. An update to PYTHONPATH is necessary though. There are still some warnings to be fixed in different modules in which an update to doc strings is necessary. I didn't find a cleaner and less intrusive way to solve this little issue. Cheers Giuseppe -- Giuseppe Vallarelli `All code is guilty until proven innocent.` www.giuseppevallarelli.com (http://www.giuseppevallarelli.com) On Saturday, February 2, 2013 at 3:32 PM, Giuseppe Vallarelli wrote: > Dear vdsm developers > I'm Giuseppe I've recently pushed a little work I've done to improve > vdsm's sphinx documentation. I've moved doc structure to the > project root folder and updated references in the different *.rst files > used. The way the import statements are used in vdsm doesn't allow > sphinx to generate documentation especially when auto directives > are used, so to try out my change set, is important to update > the python path env variable by making it pointing also to the > vdsm python package. Unluckily the modules clientIf and vm still cause > troubles when sphinx wants to extract documentation (see import > error: from vdsm import vdscli). > More work needs to be done to clean up documentation, this is > just a starting point. In the commit messages I've linked > this bug (https://bugzilla.redhat.com/show_bug.cgi?id=491467) cause I thought > it was the most appropriate. > > I do apologize for creating more commits than necessary. > I usually split my work in small chunks, but with gerrit it seems > that reviews are done on a per commit basis (I'm used to > github's pull requests). > > Cheers Giuseppe > > > -- > Giuseppe Vallarelli > > `All code is guilty until proven innocent.` > www.giuseppevallarelli.com (http://www.giuseppevallarelli.com) ___ vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
[vdsm] sphinx doc generation
Dear vdsm developers I'm Giuseppe I've recently pushed a little work I've done to improve vdsm's sphinx documentation. I've moved doc structure to the project root folder and updated references in the different *.rst files used. The way the import statements are used in vdsm doesn't allow sphinx to generate documentation especially when auto directives are used, so to try out my change set, is important to update the python path env variable by making it pointing also to the vdsm python package. Unluckily the modules clientIf and vm still cause troubles when sphinx wants to extract documentation (see import error: from vdsm import vdscli). More work needs to be done to clean up documentation, this is just a starting point. In the commit messages I've linked this bug (https://bugzilla.redhat.com/show_bug.cgi?id=491467) cause I thought it was the most appropriate. I do apologize for creating more commits than necessary. I usually split my work in small chunks, but with gerrit it seems that reviews are done on a per commit basis (I'm used to github's pull requests). Cheers Giuseppe -- Giuseppe Vallarelli `All code is guilty until proven innocent.` www.giuseppevallarelli.com (http://www.giuseppevallarelli.com) ___ vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
