Change in vdsm[ovirt-3.5]: net: skip network restoration if its physical devs are missing
Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
to review the following change.
Change subject: net: skip network restoration if its physical devs are missing ......................................................................
net: skip network restoration if its physical devs are missing
When performing network restoration on boot it is important that the operation configures as many networks as it can and that the operation does not fail so that vdsm can start.
In case a device breaks down, it is possible that the host had other networks configured that would still allow remote access after reboot.
This patch allows this use case by filtering out bond and net configuration depending on missing physical devices. It also introduces error level logs to make it very apparent when troubleshooting.
Bug-Url: https://bugzilla.redhat.com/1113091 Change-Id: I9f1370696f2398fe2c3cd9ad92424b126bcd4b7c Signed-off-by: Antoni S. Puimedon asegurap@redhat.com Reviewed-on: http://gerrit.ovirt.org/29313 Reviewed-by: Dan Kenigsberg danken@redhat.com --- M vdsm/vdsm-restore-net-config 1 file changed, 41 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/32/29832/1
diff --git a/vdsm/vdsm-restore-net-config b/vdsm/vdsm-restore-net-config index 7adfcac..f933547 100755 --- a/vdsm/vdsm-restore-net-config +++ b/vdsm/vdsm-restore-net-config @@ -22,6 +22,7 @@ import logging.config
from vdsm.config import config +from vdsm import netinfo
# Ifcfg persistence restoration from network.configurators import ifcfg @@ -61,13 +62,51 @@ configurator_cls().flush()
persistentConfig = PersistentConfig() - nets = persistentConfig.networks - bonds = persistentConfig.bonds + nets, bonds = _filter_nets_bonds(persistentConfig.networks, + persistentConfig.bonds) logging.debug('Calling setupNetworks with networks (%s) and bond (%s).', nets, bonds) setupNetworks(nets, bonds, connectivityCheck=False, _inRollback=True)
+def _filter_nets_bonds(nets, bonds): + """Returns only nets and bonds that can be configured with the devices + present in the system""" + available_nets, available_bonds = {}, {} + available_nics = netinfo.nics() + for bond, attrs in bonds.iteritems(): + available_bond_nics = [nic for nic in attrs['nics'] if + nic in available_nics] + if available_bond_nics: + available_bonds[bond] = attrs.copy() + available_bonds[bond]['nics'] = available_bond_nics + + for net, attrs in nets.iteritems(): + bond = attrs.get('bonding') + if bond is not None: + if bond not in available_bonds: + logging.error('Some of the nics required by bond "%s" (%s) ' + 'are missing. Network "%s" will not be ' + 'configured as a consequence', bond, + bonds[bond]['nics'], net) + else: + available_nets[net] = attrs + continue # Regardless of availability, the net is processed + + nic = attrs.get('nic') + if nic is not None: + if nic not in available_nics: + logging.error('Nic "%s" required by network %s is missing. ' + 'The network will not be configured', nic, net) + else: + available_nets[net] = attrs + continue # Regardless of availability, the net is processed + + # Bridge-only nics + available_nets[net] = attrs + return available_nets, available_bonds + + def _get_all_configurators(): """Returns the class objects of all the configurators in the netconf pkg""" prefix = configurators.__name__ + '.'
Antoni Segura Puimedon has posted comments on this change.
Change subject: net: skip network restoration if its physical devs are missing ......................................................................
Patch Set 1:
Passed the functional tests on el6:
rhel65_01 tests (826e722) # ./run_tests_local.sh -x functional/networkTests.py NetworkTest testAddDelBondedNetwork(kwargs=False) OK testAddDelBondedNetwork(kwargs=True) OK testAddDelNetwork(kwargs=False) OK testAddDelNetwork(kwargs=True) OK testAddNetworkBondWithManyVlans(kwargs=False) OK testAddNetworkBondWithManyVlans(kwargs=True) OK testAddNetworkManyVlans(kwargs=False) OK testAddNetworkManyVlans(kwargs=True) OK testAddNetworkVlan(kwargs=False) OK testAddNetworkVlan(kwargs=True) OK testAddNetworkVlanBond(kwargs=False) OK testAddNetworkVlanBond(kwargs=True) OK testAddVlanedBridgeless OK testAddVlanedBridgeless_oneCommand OK testAfterNetworkSetupHook OK testBeforeNetworkSetupHook OK testBondHwAddress(kwargs=False) OK testBondHwAddress(kwargs=True) OK testBrokenBridgelessNetReplacement OK testDelNetworkBondAccumulation OK testDelNetworkWithMTU(kwargs=False) OK testDelNetworkWithMTU(kwargs=True) OK testDelWithoutAdd(kwargs=False) OK testDelWithoutAdd(kwargs=True) OK testDhclientLeases(kwargs='default') OK testDhclientLeases(kwargs='local') OK testEditWithoutAdd(kwargs=False) OK testEditWithoutAdd(kwargs=True) OK testFailWithInvalidBondingName(kwargs=False) OK testFailWithInvalidBondingName(kwargs=True) OK testFailWithInvalidBridgeName OK testFailWithInvalidIpConfig OK testFailWithInvalidNic(kwargs=False) OK testFailWithInvalidNic(kwargs=True) OK testFailWithInvalidParams(kwargs=False) OK testFailWithInvalidParams(kwargs=True) OK testGetRouteDeviceTo OK testHonorBlockingDhcp OK testIPv6ConfigNetwork OK testIpLinkWrapper OK testLegacyBonds OK testLowerMtuDoesNotOverride OK testNoBridgeLeftovers OK testQosNetwork OK testReconfigureBrNetWithVanishedPort OK testRedefineBondedNetworkIPs OK testRouteExists OK testRuleExists OK testSafeNetworkConfig(kwargs=False) OK testSafeNetworkConfig(kwargs=True) OK testSetupNetworksAddBadParams(bridged=False) OK testSetupNetworksAddBadParams(bridged=True) OK testSetupNetworksAddBondWithManyVlans(kwargs=False) OK testSetupNetworksAddBondWithManyVlans(kwargs=True) OK testSetupNetworksAddDelBondedNetwork(kwargs=False) OK testSetupNetworksAddDelBondedNetwork(kwargs=True) OK testSetupNetworksAddDelDhcp(kwargs=False) OK testSetupNetworksAddDelDhcp(kwargs=True) OK testSetupNetworksAddManyVlans(kwargs=False) OK testSetupNetworksAddManyVlans(kwargs=True) OK testSetupNetworksAddNetworkToNicAfterBondBreaking(kwargs=False)OK testSetupNetworksAddNetworkToNicAfterBondBreaking(kwargs=True)OK testSetupNetworksAddNetworkToNicAfterBondResizing(kwargs=False)OK testSetupNetworksAddNetworkToNicAfterBondResizing(kwargs=True)OK testSetupNetworksAddOverExistingBond(kwargs=False) OK testSetupNetworksAddOverExistingBond(kwargs=True) OK testSetupNetworksAddVlan(kwargs=False) OK testSetupNetworksAddVlan(kwargs=True) SKIP: This test is known to break until initscripts-9.03.41-1.el6 is released to fix https://bugzilla.redhat.com/1086897 testSetupNetworksConvertVlanNetBridgeness OK testSetupNetworksDelOneOfBondNets OK testSetupNetworksKeepNetworkOnBondAfterBondResizing(kwargs=False)OK testSetupNetworksKeepNetworkOnBondAfterBondResizing(kwargs=True)OK testSetupNetworksMtus(kwargs=False) OK testSetupNetworksMtus(kwargs=True) OK testSetupNetworksMultiMTUsOverBond(kwargs=False) OK testSetupNetworksMultiMTUsOverBond(kwargs=True) OK testSetupNetworksMultiMTUsOverNic(kwargs=False) OK testSetupNetworksMultiMTUsOverNic(kwargs=True) OK testSetupNetworksNetCompatibilityBondSingleBridge OK testSetupNetworksNetCompatibilityBondSingleBridgeless OK testSetupNetworksNetCompatibilityNicSingleBridge OK testSetupNetworksNetCompatibilityNicSingleBridgeless OK testSetupNetworksNicless OK testSetupNetworksNiclessBridgeless OK testSetupNetworksOverDhcpIface OK testSetupNetworksResizeBond(kwargs=False) OK testSetupNetworksResizeBond(kwargs=True) OK testSetupNetworksStableBond(kwargs=False) OK testSetupNetworksStableBond(kwargs=True) OK testStaticSourceRouting(kwargs=False) OK testStaticSourceRouting(kwargs=True) OK testTwiceAdd(kwargs=False) OK testTwiceAdd(kwargs=True) OK testVolatileConfig(kwargs=False) OK testVolatileConfig(kwargs=True) OK
---------------------------------------------------------------------- Ran 95 tests in 530.485s
OK (SKIP=1) rhel65_01 tests (826e722) #
Antoni Segura Puimedon has posted comments on this change.
Change subject: net: skip network restoration if its physical devs are missing ......................................................................
Patch Set 1: Verified+1
did:
vdsClient -s 0 addNetwork bridge=foo bonding=bond1 nics=eth1,eth2
Then I edited /var/lib/vdsm/persistence/bonds/bond1 to:
{"nics": ["eth1", "eth20"], "options": "mode=802.3ad miimon=150"}
And eth20 does not exist in the system. Rebooted:
'bond1': {'addr': '', 'cfg': {'BONDING_OPTS': 'mode=802.3ad miimon=150', 'BRIDGE': 'foo', 'DEVICE': 'bond1', 'HOTPLUG': 'no', 'NM_CONTROLLED': 'no', 'ONBOOT': 'no'}, 'hwaddr': '00:1a:4a:0c:87:92', 'ipv4addrs': [], 'ipv6addrs': ['fe80::21a:4aff:fe0c:8792/64'], 'mtu': '1500', 'netmask': '', 'opts': {'miimon': '150', 'mode': '4'}, 'slaves': ['eth1']},
The network was successfully recreated with only the existing slave.
Then I edited /var/lib/vdsm/persistence/bonds/bond1 to:
{"nics": ["eth10", "eth20"], "options": "mode=802.3ad miimon=150"}
And neither eth10 nor eth20 exist in the system. Rebooted:
MainThread::ERROR::2014-07-11 00:53:43,100::vdsm-restore-net-config::91::root::(_filter_nets_bonds) Some of the nics required by bond "bond1" ([u'eth10', u'eth20']) are missing. Network "foo" will not be configured as a cons equence
As expected, the network was skipped.
Then removed the foo and bond1 persistent files and did:
vdsClient -s 0 addNetwork bridge=foo nics=eth1
Then I edited /var/lib/vdsm/persistence/nets/foo to:
{"nic": "eth10"}
rebooted
MainThread::ERROR::2014-07-11 00:58:33,858::vdsm-restore-net-config::100::root::(_filter_nets_bonds) Nic "eth10" required by network foo is missing. The network will not be configured
As expected, the network was skipped.
Dan Kenigsberg has posted comments on this change.
Change subject: net: skip network restoration if its physical devs are missing ......................................................................
Patch Set 1: Code-Review+2
Sandro Bonazzola has posted comments on this change.
Change subject: net: skip network restoration if its physical devs are missing ......................................................................
Patch Set 1: Code-Review+1
Yaniv Bronhaim has submitted this change and it was merged.
Change subject: net: skip network restoration if its physical devs are missing ......................................................................
net: skip network restoration if its physical devs are missing
When performing network restoration on boot it is important that the operation configures as many networks as it can and that the operation does not fail so that vdsm can start.
In case a device breaks down, it is possible that the host had other networks configured that would still allow remote access after reboot.
This patch allows this use case by filtering out bond and net configuration depending on missing physical devices. It also introduces error level logs to make it very apparent when troubleshooting.
Bug-Url: https://bugzilla.redhat.com/1113091 Change-Id: I9f1370696f2398fe2c3cd9ad92424b126bcd4b7c Signed-off-by: Antoni S. Puimedon asegurap@redhat.com Reviewed-on: http://gerrit.ovirt.org/29313 Reviewed-by: Dan Kenigsberg danken@redhat.com Reviewed-on: http://gerrit.ovirt.org/29832 Reviewed-by: Sandro Bonazzola sbonazzo@redhat.com --- M vdsm/vdsm-restore-net-config 1 file changed, 41 insertions(+), 2 deletions(-)
Approvals: Sandro Bonazzola: Looks good to me, but someone else must approve Antoni Segura Puimedon: Verified Dan Kenigsberg: Looks good to me, approved
oVirt Jenkins CI Server has posted comments on this change.
Change subject: net: skip network restoration if its physical devs are missing ......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_3.5_create-rpms_merged/27/ : SUCCESS
vdsm-patches@lists.fedorahosted.org
-
asegurap＠redhat.com -
Dan Kenigsberg -
Jenkins CI RO -
Sandro Bonazzola -
ybronhei＠redhat.com