Petr Horáček has posted comments on this change.
Change subject: net: ovs: better rollback ......................................................................
Patch Set 8:
(13 comments)
https://gerrit.ovirt.org/#/c/46907/8//COMMIT_MSG Commit Message:
Line 5: CommitDate: 2015-10-22 18:31:05 +0200 Line 6: Line 7: net: ovs: better rollback Line 8: Line 9: Untill now, OVS hook was not able to rollback after failed
Until
Done Line 10: setup of non-OVS networks. Line 11: Line 12: Now it uses after_network_setup and after_network_setup_fail Line 13: hook points to handle general rollback for all failtures
Line 9: Untill now, OVS hook was not able to rollback after failed Line 10: setup of non-OVS networks. Line 11: Line 12: Now it uses after_network_setup and after_network_setup_fail Line 13: hook points to handle general rollback for all failtures
failures
Done Line 14: (both OVS and non-OVS). Line 15: Line 16: Before OVS setup we save initial OVS configuration to a temporary Line 17: file via pickle. If an exception occurs during setupNetworks,
Line 15: Line 16: Before OVS setup we save initial OVS configuration to a temporary Line 17: file via pickle. If an exception occurs during setupNetworks, Line 18: API.py does the standard rollback. Then after_network_setup_fail Line 19: is executed and runs setupNetworks witch special option _inOVSRollback
with a
Done Line 20: which triggers OVS only rollback. OVS only rollback removes all Line 21: OVS networks and recreates them according to configuration saved in Line 22: the temprorary file. When everything is done, temporary file is Line 23: removed.
https://gerrit.ovirt.org/#/c/46907/8/vdsm_hooks/ovs/README File vdsm_hooks/ovs/README:
Line 61: 1) After ovs_before_network_setup.py:configure() calls prepare_libvirt() (which Line 62: is the last safe moment before we touch any system configuration) we save Line 63: initial network configuration into a temporary file. Line 64: 2) If an error occurs during setupNetworks(), non-OVS rolls back to the state Line 65: between OVS and non-OVS configuration. When this rollback finishes
finishes,
Done Line 66: ovs_after_network_setup_fail.py is executed. This scripts just executes Line 67: setupNetworks command with special option _inOVSRollback which tells OVS Line 68: hook to do a rollback. Line 69: 2.1) If there is a temporary file containing initial network configuration,
Line 114: | Line 115: V Line 116: +-----------------------------------------------------------------------+ Line 117: | *API.setupNetworks()* | Line 118: | | If anything bad happen, it |
happens
Done Line 119: | network setup : will trigger non-OVS rollback (which | Line 120: | | will be ignored by OVS). when it's done | Line 121: | | it will continue with | Line 122: | | after_network_setup_fail hook point. |
Line 132: |||| | Receives _inRollback=True and |||| Line 133: |||| *ovs_before_network_setup* : _inOVSRollback=True. If there |||| Line 134: |||| | is no OVS init config, we |||| Line 135: |||| | just log. If the file exists |||| Line 136: |||| | we remove all OVS networks, ||||
trailing space
Done Line 137: |||| | and recreate networks |||| Line 138: |||| | according to saved config. |||| Line 139: |||| | Then we send empty configuration |||| Line 140: |||| | back to VDSM, because of VDSM ||||
https://gerrit.ovirt.org/#/c/46907/8/vdsm_hooks/ovs/ovs_after_get_stats.py File vdsm_hooks/ovs/ovs_after_get_stats.py:
Line 1: #!/usr/bin/env python
The file has a hashbang, but its mode changed to non-executable in this com
Makefile turns it into a executable. This test does not have embedded test, to it does not need x flag. But according to your patch comment, I'm changing it. Line 2: # Copyright 2015 Red Hat, Inc. Line 3: # Line 4: # This program is free software; you can redistribute it and/or modify Line 5: # it under the terms of the GNU General Public License as published by
https://gerrit.ovirt.org/#/c/46907/8/vdsm_hooks/ovs/ovs_after_network_setup.... File vdsm_hooks/ovs/ovs_after_network_setup.py:
Line 37: elif inRollback and inOVSRollback: Line 38: hooking.log('OVS rollback is done. Removing OVS init_config backup.') Line 39: remove_init_config() Line 40: else: Line 41: hooking.log('Network setup was successfull. Removing OVS init_config '
successful
Done Line 42: 'backup.') Line 43: remove_init_config() Line 44: Line 45:
https://gerrit.ovirt.org/#/c/46907/8/vdsm_hooks/ovs/ovs_after_network_setup_... File vdsm_hooks/ovs/ovs_after_network_setup_fail.py:
Line 1: #!/usr/bin/env python
hashbang/non-executable mismatch, as with other files
Makefile turns it into executable. We have x flag only on hook files with simple tests (well ovs_after_get_stats.py is executable and has not test, but this is fixed in one of following patches). Line 2: # Copyright 2015 Red Hat, Inc. Line 3: # Line 4: # This program is free software; you can redistribute it and/or modify Line 5: # it under the terms of the GNU General Public License as published by
Line 26: Line 27: def main(): Line 28: setup_nets_config = hooking.read_json() Line 29: in_rollback = setup_nets_config['request']['options'].get('_inRollback', Line 30: False)
dict.get() would just return None.
Done Line 31: if in_rollback: Line 32: hooking.log('Failed while trying to rollback.') Line 33: else: Line 34: hooking.log('Configuration failed. In this point, non-OVS rollback '
Line 30: False) Line 31: if in_rollback: Line 32: hooking.log('Failed while trying to rollback.') Line 33: else: Line 34: hooking.log('Configuration failed. In this point, non-OVS rollback '
At
Done Line 35: 'should be done. Executing OVS rollback.') Line 36: supervdsm.getProxy().setupNetworks( Line 37: {}, {}, {'connectivityCheck': False, '_inRollback': True, Line 38: '_inOVSRollback': True})
https://gerrit.ovirt.org/#/c/46907/8/vdsm_hooks/ovs/ovs_before_network_setup... File vdsm_hooks/ovs/ovs_before_network_setup.py:
Line 78: Line 79: destroy_ovs_bridge() Line 80: for net, attrs in running_config.networks.items(): Line 81: if is_ovs_network(attrs): Line 82: running_config.networks.pop(net)
Doesn't Python complain when you modify a dictionary while iterating it? Yo
I'm not iterating that dict, .items() created a copy of it. Anyways, in next patch I use six everywhere. Line 83: for bond, attrs in running_config.bonds.items(): Line 84: if is_ovs_bond(attrs): Line 85: running_config.bonds.pop(bond) Line 86: running_config.save()
Line 92: hooking.log('No needed OVS changes to be done.') Line 93: else: Line 94: hooking.log('Remove OVS networks.') Line 95: _remove_ovs_nets(initial_config, running_config) Line 96: hooking.log('Reconfigure OVS networks according to initial_config.')
Reconfiguring
Done Line 97: _configure(initial_config.networks, initial_config.bonds, Line 98: running_config, save_init=False) Line 99: Line 100: