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.py 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_fail.py 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.py 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: -- To view, visit https://gerrit.ovirt.org/46907 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8f6b63d03bb9579e260bfad1686047a431f69543 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Ido Barkan <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda <[email protected]> Gerrit-Reviewer: Petr Horáček <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
