Skip to content

vpc: fix ips on wrong interfaces after rebooting vpc vrs #4467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yadvr merged 3 commits into from
Nov 20, 2020
Merged

vpc: fix ips on wrong interfaces after rebooting vpc vrs #4467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
522f4eb
vpc: fix ips on wrong interfaces after rebooting vpc vrs
ustcweizhou Nov 10, 2020
eba9c86
#4467: Rename to updateNicWithDeviceId
ustcweizhou Nov 13, 2020
f603816
CLSTACK-8923 vr: Force a restart of keepalived if conntrackd is not r…
ustcweizhou Nov 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ public boolean finalizeCommandsOnStart(final Commands cmds, final VirtualMachine
vlanMacAddress.put(vlanTag, routerNic.getMacAddress());
}
}
int deviceId = 1; //Public and Guest networks start from device_id = 1

final List<Command> usageCmds = new ArrayList<Command>();

Expand All @@ -347,7 +348,8 @@ public boolean finalizeCommandsOnStart(final Commands cmds, final VirtualMachine
// add VPC router to public networks
final List<PublicIp> sourceNat = new ArrayList<PublicIp>(1);
for (final Pair<Nic, Network> nicNtwk : publicNics) {
final Nic publicNic = nicNtwk.first();
final Nic publicNic = updateNicWithDeviceId(nicNtwk.first().getId(), deviceId);
deviceId ++;
final Network publicNtwk = nicNtwk.second();
final IPAddressVO userIp = _ipAddressDao.findByIpAndSourceNetworkId(publicNtwk.getId(), publicNic.getIPv4Address());

Expand Down Expand Up @@ -385,7 +387,8 @@ public boolean finalizeCommandsOnStart(final Commands cmds, final VirtualMachine

// add VPC router to guest networks
for (final Pair<Nic, Network> nicNtwk : guestNics) {
final Nic guestNic = nicNtwk.first();
final Nic guestNic = updateNicWithDeviceId(nicNtwk.first().getId(), deviceId);
deviceId ++;
// plug guest nic
final PlugNicCommand plugNicCmd = new PlugNicCommand(_nwHelper.getNicTO(domainRouterVO, guestNic.getNetworkId(), null), domainRouterVO.getInstanceName(), domainRouterVO.getType(), details);
cmds.addCommand(plugNicCmd);
Expand Down Expand Up @@ -834,4 +837,11 @@ public boolean postStateTransitionEvent(final StateMachine2.Transition<State, Vi
// once from VpcVirtualNetworkApplianceManagerImpl and once from VirtualNetworkApplianceManagerImpl itself
return true;
}

private Nic updateNicWithDeviceId(final long nicId, int deviceId) {
NicVO nic = _nicDao.findById(nicId);
nic.setDeviceId(deviceId);
_nicDao.update(nic.getId(), nic);
return nic;
}
Copy link
Contributor

@sureshanaparti sureshanaparti Nov 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

method name is not clear, Either define & use separate methods (for "updating nic device id" and "getting nic with device id"), or change the name appropriately.

Copy link
Contributor

@DaanHoogland DaanHoogland Nov 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getNicAndUpdtaeDeviceId() might have been more accurate but this is clearly stating what it does on the tin (to me) , Can you be more explicit about what you want @sureshanaparti ?

Copy link
Contributor

@sureshanaparti sureshanaparti Nov 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getNicAndUpdtaeDeviceId() might have been more accurate but this is clearly stating what it does on the tin (to me) , Can you be more explicit about what you want @sureshanaparti ?

simply "updateNicWithDeviceId()" returning nic would be ok.

Copy link
Member

@weizhouapache weizhouapache Nov 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sureshanaparti updateNicWithDeviceId() looks good to me. @DaanHoogland what's your opinion ?

Copy link
Contributor

@DaanHoogland DaanHoogland Nov 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, fine. the only problem i have is with my own spelling; getNicAndUpdtaeDeviceId() misses a 'k' ;)

Copy link
Member

@weizhouapache weizhouapache Nov 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sureshanaparti @DaanHoogland thanks guys. code is changed.

}
2 changes: 1 addition & 1 deletion systemvm/debian/opt/cloud/bin/cs/CsRedundant.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def _redundant_on(self):
force_keepalived_restart = False
proc = CsProcess(['/etc/conntrackd/conntrackd.conf'])

if not proc.find() and not is_equals:
if not proc.find() or not is_equals:
CsHelper.copy(conntrackd_template_conf, self.CONNTRACKD_CONF)
CsHelper.service("conntrackd", "restart")
force_keepalived_restart = True
Expand Down
3 changes: 3 additions & 0 deletions systemvm/debian/opt/cloud/bin/setup/bootstrap.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ config_guest() {
for i in {1..60}; do
if [ -s $CMDLINE ]; then
log_it "Received a new non-empty cmdline file from qemu-guest-agent"
# Remove old configuration files in /etc/cloudstack if VR is booted from cloudstack
rm -rf /etc/cloudstack/*.json
log_it "Booting from cloudstack, remove old configuration files in /etc/cloudstack/"
break
fi
sleep 1
Expand Down
34 changes: 34 additions & 0 deletions test/integration/component/test_multiple_subnets_in_isolated_network.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"""

from nose.plugins.attrib import attr
from marvin.cloudstackAPI import rebootRouter
from marvin.cloudstackTestCase import cloudstackTestCase, unittest
from marvin.lib.utils import (validateList,
get_host_credentials,
Expand Down Expand Up @@ -106,6 +107,23 @@ def tearDown(cls):
raise Exception("Warning: Exception during cleanup : %s" % e)
return

def get_router(self, router_id):
routers = list_routers(
self.apiclient,
id=router_id,
listall=True)
self.assertEqual(
isinstance(routers, list),
True,
"Check for list routers response return valid data"
)
self.assertNotEqual(
len(routers),
0,
"Check list router response"
)
return routers[0]

def get_routers(self, network_id):
routers = list_routers(
self.apiclient,
Expand Down Expand Up @@ -672,6 +690,22 @@ def test_01_acquire_public_ips_in_isolated_network_with_single_vr(self):
self.verify_ip_address_in_router(router, host, ipaddress_5.ipaddress.ipaddress, "eth4", False)
self.verify_ip_address_in_router(router, host, ipaddress_6.ipaddress.ipaddress, "eth4", True)

# reboot router
for router in routers:
cmd = rebootRouter.rebootRouterCmd()
cmd.id = router.id
self.apiclient.rebootRouter(cmd)
router = self.get_router(router.id)
host = self.get_router_host(router)
self.verify_network_interfaces_in_router(router, host, "eth0,eth1,eth2,eth3,")
guestIp, controlIp, sourcenatIp = self.get_router_ips(router)
self.verify_ip_address_in_router(router, host, guestIp, "eth0", True)
self.verify_ip_address_in_router(router, host, controlIp, "eth1", True)
self.verify_ip_address_in_router(router, host, sourcenatIp, "eth2", True)
self.verify_ip_address_in_router(router, host, ipaddress_4.ipaddress.ipaddress, "eth3", False)
self.verify_ip_address_in_router(router, host, ipaddress_5.ipaddress.ipaddress, "eth3", False)
self.verify_ip_address_in_router(router, host, ipaddress_6.ipaddress.ipaddress, "eth3", True)

# 20. restart network with cleanup
self.network1.restart(self.apiclient, cleanup=True)
routers = self.get_routers(self.network1.id)
Expand Down
34 changes: 34 additions & 0 deletions test/integration/component/test_multiple_subnets_in_isolated_network_rvr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"""

from nose.plugins.attrib import attr
from marvin.cloudstackAPI import rebootRouter
from marvin.cloudstackTestCase import cloudstackTestCase, unittest
from marvin.lib.utils import (validateList,
get_host_credentials,
Expand Down Expand Up @@ -106,6 +107,23 @@ def tearDown(cls):
raise Exception("Warning: Exception during cleanup : %s" % e)
return

def get_router(self, router_id):
routers = list_routers(
self.apiclient,
id=router_id,
listall=True)
self.assertEqual(
isinstance(routers, list),
True,
"Check for list routers response return valid data"
)
self.assertNotEqual(
len(routers),
0,
"Check list router response"
)
return routers[0]

def get_routers(self, network_id):
routers = list_routers(
self.apiclient,
Expand Down Expand Up @@ -672,6 +690,22 @@ def test_02_acquire_public_ips_in_isolated_network_with_redundant_vrs(self):
self.verify_ip_address_in_router(router, host, ipaddress_5.ipaddress.ipaddress, "eth4", False)
self.verify_ip_address_in_router(router, host, ipaddress_6.ipaddress.ipaddress, "eth4", True)

# reboot router
for router in routers:
cmd = rebootRouter.rebootRouterCmd()
cmd.id = router.id
self.apiclient.rebootRouter(cmd)
router = self.get_router(router.id)
host = self.get_router_host(router)
self.verify_network_interfaces_in_router(router, host, "eth0,eth1,eth2,eth3,")
guestIp, controlIp, sourcenatIp = self.get_router_ips(router)
self.verify_ip_address_in_router(router, host, guestIp, "eth0", True)
self.verify_ip_address_in_router(router, host, controlIp, "eth1", True)
self.verify_ip_address_in_router(router, host, sourcenatIp, "eth2", True)
self.verify_ip_address_in_router(router, host, ipaddress_4.ipaddress.ipaddress, "eth3", False)
self.verify_ip_address_in_router(router, host, ipaddress_5.ipaddress.ipaddress, "eth3", False)
self.verify_ip_address_in_router(router, host, ipaddress_6.ipaddress.ipaddress, "eth3", True)

# 20. restart network with cleanup
self.network1.restart(self.apiclient, cleanup=True)
routers = self.get_routers(self.network1.id)
Expand Down
33 changes: 33 additions & 0 deletions test/integration/component/test_multiple_subnets_in_vpc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"""

from nose.plugins.attrib import attr
from marvin.cloudstackAPI import rebootRouter
from marvin.cloudstackTestCase import cloudstackTestCase, unittest
from marvin.lib.utils import (validateList,
get_host_credentials,
Expand Down Expand Up @@ -106,6 +107,23 @@ def tearDown(cls):
raise Exception("Warning: Exception during cleanup : %s" % e)
return

def get_router(self, router_id):
routers = list_routers(
self.apiclient,
id=router_id,
listall=True)
self.assertEqual(
isinstance(routers, list),
True,
"Check for list routers response return valid data"
)
self.assertNotEqual(
len(routers),
0,
"Check list router response"
)
return routers[0]

def get_routers(self, network_id):
routers = list_routers(
self.apiclient,
Expand Down Expand Up @@ -790,6 +808,21 @@ def test_03_acquire_public_ips_in_vpc_with_single_vr(self):
self.verify_ip_address_in_router(router, host, tier2_Ip, "eth4", True)
self.verify_ip_address_in_router(router, host, ipaddress_6.ipaddress.ipaddress, "eth5", True)

# reboot router
for router in routers:
cmd = rebootRouter.rebootRouterCmd()
cmd.id = router.id
self.apiclient.rebootRouter(cmd)
router = self.get_router(router.id)
host = self.get_router_host(router)
self.verify_network_interfaces_in_router(router, host, "eth0,eth1,eth2,eth3,eth4,")
controlIp, sourcenatIp, tier1_Ip, tier2_Ip = self.get_vpc_router_ips(router)
self.verify_ip_address_in_router(router, host, controlIp, "eth0", True)
self.verify_ip_address_in_router(router, host, sourcenatIp, "eth1", True)
self.verify_ip_address_in_router(router, host, ipaddress_6.ipaddress.ipaddress, "eth2", True)
self.verify_ip_address_in_router(router, host, tier1_Ip, "eth3", True)
self.verify_ip_address_in_router(router, host, tier2_Ip, "eth4", True)

# 23. restart VPC with cleanup
self.vpc1.restart(self.apiclient, cleanup=True)
routers = self.get_vpc_routers(self.vpc1.id)
Expand Down
33 changes: 33 additions & 0 deletions test/integration/component/test_multiple_subnets_in_vpc_rvr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"""

from nose.plugins.attrib import attr
from marvin.cloudstackAPI import rebootRouter
from marvin.cloudstackTestCase import cloudstackTestCase, unittest
from marvin.lib.utils import (validateList,
get_host_credentials,
Expand Down Expand Up @@ -106,6 +107,23 @@ def tearDown(cls):
raise Exception("Warning: Exception during cleanup : %s" % e)
return

def get_router(self, router_id):
routers = list_routers(
self.apiclient,
id=router_id,
listall=True)
self.assertEqual(
isinstance(routers, list),
True,
"Check for list routers response return valid data"
)
self.assertNotEqual(
len(routers),
0,
"Check list router response"
)
return routers[0]

def get_routers(self, network_id):
routers = list_routers(
self.apiclient,
Expand Down Expand Up @@ -790,6 +808,21 @@ def test_04_acquire_public_ips_in_vpc_with_redundant_vrs(self):
self.verify_ip_address_in_router(router, host, tier2_Ip, "eth4", True)
self.verify_ip_address_in_router(router, host, ipaddress_6.ipaddress.ipaddress, "eth5", True)

# reboot router
for router in routers:
cmd = rebootRouter.rebootRouterCmd()
cmd.id = router.id
self.apiclient.rebootRouter(cmd)
router = self.get_router(router.id)
host = self.get_router_host(router)
self.verify_network_interfaces_in_router(router, host, "eth0,eth1,eth2,eth3,eth4,")
controlIp, sourcenatIp, tier1_Ip, tier2_Ip = self.get_vpc_router_ips(router)
self.verify_ip_address_in_router(router, host, controlIp, "eth0", True)
self.verify_ip_address_in_router(router, host, sourcenatIp, "eth1", True)
self.verify_ip_address_in_router(router, host, ipaddress_6.ipaddress.ipaddress, "eth2", True)
self.verify_ip_address_in_router(router, host, tier1_Ip, "eth3", True)
self.verify_ip_address_in_router(router, host, tier2_Ip, "eth4", True)

# 23. restart VPC with cleanup
self.vpc1.restart(self.apiclient, cleanup=True)
routers = self.get_vpc_routers(self.vpc1.id)
Expand Down