Skip to content

Commit 42863fd

Browse files
Dogface2kDogface2k
committed
OVS: contain malformed topology callback failures
1 parent b8f4ab1 commit 42863fd

2 files changed

Lines changed: 145 additions & 17 deletions

File tree

‎plugins/network-elements/ovs/src/main/java/com/cloud/network/ovs/OvsTunnelManagerImpl.java‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -694,19 +694,24 @@ private void handleVmStateChange(VMInstanceVO vm) {
694694
continue;
695695
}
696696

697-
// get the list of hosts on which VPC spans (i.e hosts that need to be aware of VPC topology change update)
698-
List<Long> vpcSpannedHostIds = _ovsNetworkToplogyGuru.getVpcSpannedHosts(vpcId);
699-
String bridgeName=generateBridgeNameForVpc(vpcId);
700-
701-
OvsVpcPhysicalTopologyConfigCommand topologyConfigCommand = prepareVpcTopologyUpdate(vpcId);
702-
topologyConfigCommand.setSequenceNumber(getNextTopologyUpdateSequenceNumber(vpcId));
703-
704-
// send topology change update to VPC spanned hosts
705-
for (Long id: vpcSpannedHostIds) {
706-
if (!sendVpcTopologyChangeUpdate(topologyConfigCommand, id, bridgeName)) {
707-
logger.debug("Failed to send VPC topology change update to host : " + id + ". Moving on " +
708-
"with rest of the host update.");
697+
try {
698+
// get the list of hosts on which VPC spans (i.e hosts that need to be aware of VPC topology change update)
699+
List<Long> vpcSpannedHostIds = _ovsNetworkToplogyGuru.getVpcSpannedHosts(vpcId);
700+
String bridgeName=generateBridgeNameForVpc(vpcId);
701+
702+
OvsVpcPhysicalTopologyConfigCommand topologyConfigCommand = prepareVpcTopologyUpdate(vpcId);
703+
topologyConfigCommand.setSequenceNumber(getNextTopologyUpdateSequenceNumber(vpcId));
704+
705+
// send topology change update to VPC spanned hosts
706+
for (Long id: vpcSpannedHostIds) {
707+
if (!sendVpcTopologyChangeUpdate(topologyConfigCommand, id, bridgeName)) {
708+
logger.debug("Failed to send VPC topology change update to host : " + id + ". Moving on " +
709+
"with rest of the host update.");
710+
}
709711
}
712+
} catch (RuntimeException e) {
713+
logger.error("Failed to update OVS distributed-router topology for VPC {} after VM {} changed state",
714+
vpcId, vm.getId(), e);
710715
}
711716
}
712717
}
@@ -764,19 +769,19 @@ OvsVpcPhysicalTopologyConfigCommand prepareVpcTopologyUpdate(long vpcId) {
764769
vpc.getUuid(), network.getUuid()));
765770
}
766771
String key = network.getBroadcastUri().getAuthority();
767-
String[] parts = StringUtils.split(key, '.');
768-
if (parts == null || parts.length != 2 || !String.valueOf(vpcId).equals(parts[0])) {
772+
String expectedPrefix = vpcId + ".";
773+
if (key == null || !key.startsWith(expectedPrefix) || key.indexOf('.', expectedPrefix.length()) >= 0) {
769774
throw new CloudRuntimeException(String.format(
770775
"OVS distributed-router network %s has invalid broadcast key %s for VPC %s",
771776
network.getUuid(), key, vpc.getUuid()));
772777
}
773-
long greKey;
778+
int greKey;
774779
try {
775-
greKey = Long.parseLong(parts[1]);
780+
greKey = Integer.parseInt(key.substring(expectedPrefix.length()));
776781
} catch (NumberFormatException e) {
777782
throw new CloudRuntimeException(String.format(
778783
"OVS distributed-router network %s has non-numeric GRE key %s",
779-
network.getUuid(), parts[1]), e);
784+
network.getUuid(), key.substring(expectedPrefix.length())), e);
780785
}
781786
NicVO nic = _nicDao.findByIp4AddressAndNetworkId(network.getGateway(), network.getId());
782787
if (nic == null) {

‎plugins/network-elements/ovs/src/test/java/com/cloud/network/ovs/OvsTunnelManagerImplTest.java‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import com.cloud.network.dao.NetworkDao;
4242
import com.cloud.network.dao.NetworkVO;
4343
import com.cloud.network.ovs.dao.VpcDistributedRouterSeqNoDao;
44+
import com.cloud.network.ovs.dao.VpcDistributedRouterSeqNoVO;
4445
import com.cloud.network.vpc.VpcManager;
4546
import com.cloud.network.vpc.VpcVO;
4647
import com.cloud.network.vpc.dao.VpcDao;
@@ -61,6 +62,7 @@ public class OvsTunnelManagerImplTest {
6162
private VpcManager vpcManager;
6263
private OvsNetworkTopologyGuru topologyGuru;
6364
private NicDao nicDao;
65+
private VpcDistributedRouterSeqNoVO sequenceNumber;
6466

6567
@Before
6668
public void setUp() {
@@ -167,6 +169,68 @@ public void testPostStateTransitionEventContinuesAfterNonOvsVpc() {
167169
verify(vpcDao).findById(SECOND_VPC_ID);
168170
}
169171

172+
@Test
173+
public void testPostStateTransitionEventContainsMalformedOvsTopologyAndContinues() {
174+
VpcVO firstVpc = mock(VpcVO.class);
175+
VpcVO secondVpc = mock(VpcVO.class);
176+
VMInstanceVO vm = mock(VMInstanceVO.class);
177+
@SuppressWarnings("unchecked")
178+
StateMachine2.Transition<VirtualMachine.State, VirtualMachine.Event> transition = mock(StateMachine2.Transition.class);
179+
when(vm.getId()).thenReturn(11L);
180+
when(topologyGuru.getVpcIdsVmIsPartOf(11L)).thenReturn(List.of(VPC_ID, SECOND_VPC_ID));
181+
when(vpcDao.findById(VPC_ID)).thenReturn(firstVpc);
182+
when(vpcDao.findById(SECOND_VPC_ID)).thenReturn(secondVpc);
183+
when(firstVpc.usesDistributedRouter()).thenReturn(true);
184+
when(secondVpc.usesDistributedRouter()).thenReturn(true);
185+
when(vpcManager.isProviderSupportServiceInVpc(VPC_ID, Network.Service.Connectivity, Network.Provider.Ovs))
186+
.thenReturn(true);
187+
when(vpcManager.isProviderSupportServiceInVpc(SECOND_VPC_ID, Network.Service.Connectivity, Network.Provider.Ovs))
188+
.thenReturn(false);
189+
when(transition.getCurrentState()).thenReturn(VirtualMachine.State.Starting);
190+
when(transition.getEvent()).thenReturn(VirtualMachine.Event.OperationSucceeded);
191+
when(transition.getToState()).thenReturn(VirtualMachine.State.Running);
192+
Network malformedNetwork = mock(Network.class);
193+
when(topologyGuru.getVpcSpannedHosts(VPC_ID)).thenReturn(Collections.emptyList());
194+
when(topologyGuru.getAllActiveVmsInVpc(VPC_ID)).thenReturn(Collections.emptyList());
195+
doReturn(List.of(malformedNetwork)).when(vpcManager).getVpcNetworks(VPC_ID);
196+
when(firstVpc.getUuid()).thenReturn("vpc-uuid");
197+
when(firstVpc.getCidr()).thenReturn("10.0.0.0/16");
198+
when(malformedNetwork.getUuid()).thenReturn("network-uuid");
199+
when(malformedNetwork.getBroadcastDomainType()).thenReturn(BroadcastDomainType.NSX);
200+
201+
assertTrue(manager.postStateTransitionEvent(transition, vm, true, null));
202+
203+
verify(vpcDao).findById(SECOND_VPC_ID);
204+
}
205+
206+
@Test
207+
public void testPostStateTransitionEventBuildsTopologyForOvsVpc() {
208+
VpcVO vpc = mock(VpcVO.class);
209+
VMInstanceVO vm = mock(VMInstanceVO.class);
210+
@SuppressWarnings("unchecked")
211+
StateMachine2.Transition<VirtualMachine.State, VirtualMachine.Event> transition = mock(StateMachine2.Transition.class);
212+
when(vm.getId()).thenReturn(11L);
213+
when(topologyGuru.getVpcIdsVmIsPartOf(11L)).thenReturn(List.of(VPC_ID));
214+
when(vpcDao.findById(VPC_ID)).thenReturn(vpc);
215+
when(vpc.usesDistributedRouter()).thenReturn(true);
216+
when(vpc.getUuid()).thenReturn("vpc-uuid");
217+
when(vpc.getCidr()).thenReturn("10.0.0.0/16");
218+
when(vpcManager.isProviderSupportServiceInVpc(VPC_ID, Network.Service.Connectivity, Network.Provider.Ovs))
219+
.thenReturn(true);
220+
when(transition.getCurrentState()).thenReturn(VirtualMachine.State.Starting);
221+
when(transition.getEvent()).thenReturn(VirtualMachine.Event.OperationSucceeded);
222+
when(transition.getToState()).thenReturn(VirtualMachine.State.Running);
223+
when(topologyGuru.getVpcSpannedHosts(VPC_ID)).thenReturn(Collections.emptyList());
224+
when(topologyGuru.getAllActiveVmsInVpc(VPC_ID)).thenReturn(Collections.emptyList());
225+
doReturn(Collections.emptyList()).when(vpcManager).getVpcNetworks(VPC_ID);
226+
prepareSequenceNumber(VPC_ID);
227+
228+
assertTrue(manager.postStateTransitionEvent(transition, vm, true, null));
229+
230+
verify(vpcManager).getVpcNetworks(VPC_ID);
231+
verify(manager._vpcDrSeqNoDao).update(1L, sequenceNumber);
232+
}
233+
170234
@Test
171235
public void testNetworkAclSubscriberIgnoresNsxDistributedVpc() {
172236
VpcVO vpc = mock(VpcVO.class);
@@ -183,6 +247,28 @@ public void testNetworkAclSubscriberIgnoresNsxDistributedVpc() {
183247
verify(vpcManager, never()).getVpcNetworks(anyLong());
184248
}
185249

250+
@Test
251+
public void testNetworkAclSubscriberBuildsPolicyForOvsVpc() {
252+
VpcVO vpc = mock(VpcVO.class);
253+
NetworkVO network = mock(NetworkVO.class);
254+
when(network.getVpcId()).thenReturn(VPC_ID);
255+
when(vpcDao.findById(VPC_ID)).thenReturn(vpc);
256+
when(vpc.usesDistributedRouter()).thenReturn(true);
257+
when(vpc.getUuid()).thenReturn("vpc-uuid");
258+
when(vpc.getCidr()).thenReturn("10.0.0.0/16");
259+
when(vpcManager.isProviderSupportServiceInVpc(VPC_ID, Network.Service.Connectivity, Network.Provider.Ovs))
260+
.thenReturn(true);
261+
doReturn(List.of(network)).when(vpcManager).getVpcNetworks(VPC_ID);
262+
when(network.getNetworkACLId()).thenReturn(null);
263+
when(topologyGuru.getVpcSpannedHosts(VPC_ID)).thenReturn(Collections.emptyList());
264+
prepareSequenceNumber(VPC_ID);
265+
266+
manager.new NetworkAclEventsSubscriber().onPublishMessage("sender", "Network_ACL_Replaced", network);
267+
268+
verify(vpcManager).getVpcNetworks(VPC_ID);
269+
verify(manager._vpcDrSeqNoDao).update(1L, sequenceNumber);
270+
}
271+
186272
@Test
187273
public void testPrepareVpcTopologyUpdateRejectsNonVswitchTier() {
188274
VpcVO vpc = mock(VpcVO.class);
@@ -214,6 +300,34 @@ public void testPrepareVpcTopologyUpdateRejectsNonNumericGreKey() {
214300
assertThrows(CloudRuntimeException.class, () -> manager.prepareVpcTopologyUpdate(VPC_ID));
215301
}
216302

303+
@Test
304+
public void testPrepareVpcTopologyUpdateRejectsRepeatedDelimiterInBroadcastKey() {
305+
prepareVswitchNetwork("7..123");
306+
307+
assertThrows(CloudRuntimeException.class, () -> manager.prepareVpcTopologyUpdate(VPC_ID));
308+
}
309+
310+
@Test
311+
public void testPrepareVpcTopologyUpdateRejectsLeadingDelimiterInBroadcastKey() {
312+
prepareVswitchNetwork(".7.123");
313+
314+
assertThrows(CloudRuntimeException.class, () -> manager.prepareVpcTopologyUpdate(VPC_ID));
315+
}
316+
317+
@Test
318+
public void testPrepareVpcTopologyUpdateRejectsTrailingDelimiterInBroadcastKey() {
319+
prepareVswitchNetwork("7.123.");
320+
321+
assertThrows(CloudRuntimeException.class, () -> manager.prepareVpcTopologyUpdate(VPC_ID));
322+
}
323+
324+
@Test
325+
public void testPrepareVpcTopologyUpdateRejectsGreKeyOutsideIntegerRange() {
326+
prepareVswitchNetwork("7.2147483648");
327+
328+
assertThrows(CloudRuntimeException.class, () -> manager.prepareVpcTopologyUpdate(VPC_ID));
329+
}
330+
217331
@Test
218332
public void testPrepareVpcTopologyUpdateRejectsMissingGatewayNic() {
219333
prepareVswitchNetwork("7.123");
@@ -253,4 +367,13 @@ private Network prepareVswitchNetwork(String broadcastKey) {
253367
when(network.getBroadcastUri()).thenReturn(BroadcastDomainType.Vswitch.toUri(broadcastKey));
254368
return network;
255369
}
370+
371+
private void prepareSequenceNumber(long vpcId) {
372+
sequenceNumber = mock(VpcDistributedRouterSeqNoVO.class);
373+
when(sequenceNumber.getId()).thenReturn(1L);
374+
when(sequenceNumber.getTopologyUpdateSequenceNo()).thenReturn(1L);
375+
when(sequenceNumber.getPolicyUpdateSequenceNo()).thenReturn(1L);
376+
when(manager._vpcDrSeqNoDao.findByVpcId(vpcId)).thenReturn(sequenceNumber);
377+
when(manager._vpcDrSeqNoDao.lockRow(1L, true)).thenReturn(sequenceNumber);
378+
}
256379
}

0 commit comments

Comments
 (0)