Skip to content

Commit cdf3135

Browse files
sudo87sudo87
committed
Fix VPC public gateway rate not applied to VR public NIC, normalize null/0 to -1 in responses
The VPC-aware branch of NetworkModelImpl.getNetworkRate() resolved the router's VPC by scanning its sibling NICs for a Guest one. During initial VR deployment the public NIC is allocated before the guest NIC is persisted, so the scan found nothing and silently fell back to unlimited instead of the VPC offering's rate. Resolve the VPC from the router's own vpc_id instead, which is set at router creation and doesn't depend on NIC allocation order. Also align publicnetworkrate in VpcResponse and VpcOfferingResponse with the existing networkrate convention: always return a normalized value (-1 for unlimited) instead of a raw nullable/zero column value, and show "Unlimited" in the UI for the VPC and VPC offering views the same way NIC/network rate already does.
1 parent 7a4e287 commit cdf3135

6 files changed

Lines changed: 58 additions & 22 deletions

File tree

‎server/src/main/java/com/cloud/api/ApiResponseHelper.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3602,10 +3602,8 @@ public VpcResponse createVpcResponse(ResponseView view, Vpc vpc) {
36023602
response.setVpcOfferingConserveMode(voff.isConserveMode());
36033603
}
36043604
VpcDetailVO publicNetworkRateDetail = vpcDetailsDao.findDetail(vpc.getId(), ApiConstants.PUBLIC_NETWORK_RATE);
3605-
if (publicNetworkRateDetail != null) {
3606-
int publicNetworkRate = NumberUtils.toInt(publicNetworkRateDetail.getValue(), -1);
3607-
response.setPublicNetworkRate(publicNetworkRate > 0 ? publicNetworkRate : -1);
3608-
}
3605+
Integer publicNetworkRate = publicNetworkRateDetail != null ? NumberUtils.toInt(publicNetworkRateDetail.getValue(), -1) : null;
3606+
response.setPublicNetworkRate((publicNetworkRate == null || publicNetworkRate <= 0) ? -1 : publicNetworkRate);
36093607
response.setCidr(vpc.getCidr());
36103608
response.setRestartRequired(vpc.isRestartRequired());
36113609
response.setNetworkDomain(vpc.getNetworkDomain());

‎server/src/main/java/com/cloud/api/query/dao/VpcOfferingJoinDaoImpl.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ public VpcOfferingResponse newVpcOfferingResponse(VpcOffering offering) {
7878
offeringResponse.setSpecifyAsNumber(offering.isSpecifyAsNumber());
7979
}
8080
offeringResponse.setConserveMode(offering.isConserveMode());
81-
offeringResponse.setPublicNetworkRate(offering.getPublicNetworkRate());
81+
Integer pubNetworkRate = offering.getPublicNetworkRate();
82+
offeringResponse.setPublicNetworkRate((pubNetworkRate == null || pubNetworkRate <= 0 ) ? -1 : pubNetworkRate);
8283
if (offering instanceof VpcOfferingJoinVO) {
8384
VpcOfferingJoinVO offeringJoinVO = (VpcOfferingJoinVO) offering;
8485
offeringResponse.setDomainId(offeringJoinVO.getDomainUuid());

‎server/src/main/java/com/cloud/network/NetworkModelImpl.java‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -143,13 +143,15 @@
143143
import com.cloud.utils.db.SearchCriteria.Op;
144144
import com.cloud.utils.exception.CloudRuntimeException;
145145
import com.cloud.utils.net.NetUtils;
146+
import com.cloud.vm.DomainRouterVO;
146147
import com.cloud.vm.Nic;
147148
import com.cloud.vm.NicProfile;
148149
import com.cloud.vm.NicVO;
149150
import com.cloud.vm.VMInstanceVO;
150151
import com.cloud.vm.VirtualMachine;
151152
import com.cloud.vm.VirtualMachine.Type;
152153
import com.cloud.vm.VirtualMachineManager;
154+
import com.cloud.vm.dao.DomainRouterDao;
153155
import com.cloud.vm.dao.NicDao;
154156
import com.cloud.vm.dao.NicSecondaryIpDao;
155157
import com.cloud.vm.dao.VMInstanceDao;
@@ -194,6 +196,8 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi
194196
VpcDao vpcDao;
195197
@Inject
196198
VpcOfferingServiceMapDao _vpcOffSvcMapDao;
199+
@Inject
200+
DomainRouterDao _routerDao;
197201

198202
private List<NetworkElement> networkElements;
199203

@@ -1235,16 +1239,19 @@ public Integer getNetworkRate(long networkId, Long vmId) {
12351239
return _configMgr.getNetworkOfferingNetworkRate(network.getNetworkOfferingId(), network.getDataCenterId());
12361240
}
12371241
} else if (TrafficType.Public.equals(network.getTrafficType())) {
1242+
// Use the router's own vpc_id: the guest NIC isn't persisted yet when this runs for the public NIC during initial VR deployment.
1243+
final DomainRouterVO routerVO = _routerDao.findById(vmId);
1244+
final Long vpcId = routerVO != null ? routerVO.getVpcId() : null;
1245+
if (vpcId != null) {
1246+
final Vpc vpc = vpcDao.findById(vpcId);
1247+
if (vpc != null) {
1248+
return _configMgr.getVpcOfferingNetworkRate(vpc.getVpcOfferingId(), network.getDataCenterId());
1249+
}
1250+
}
12381251
List<NicVO> routerNics = _nicDao.listByVmId(vmId);
12391252
for (final Nic routerNic : routerNics) {
12401253
final NetworkVO nw = _networksDao.findById(routerNic.getNetworkId());
12411254
if (TrafficType.Guest.equals(nw.getTrafficType())) {
1242-
if (nw.getVpcId() != null) {
1243-
final Vpc vpc = vpcDao.findById(nw.getVpcId());
1244-
if (vpc != null) {
1245-
return _configMgr.getVpcOfferingNetworkRate(vpc.getVpcOfferingId(), network.getDataCenterId());
1246-
}
1247-
}
12481255
return _configMgr.getNetworkOfferingNetworkRate(nw.getNetworkOfferingId(), network.getDataCenterId());
12491256
}
12501257
}

‎server/src/test/java/com/cloud/network/NetworkModelImplTest.java‎

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,13 @@
7676
import com.cloud.utils.Pair;
7777
import com.cloud.utils.db.EntityManager;
7878
import com.cloud.utils.net.Ip;
79+
import com.cloud.vm.DomainRouterVO;
7980
import com.cloud.vm.Nic;
8081
import com.cloud.vm.NicProfile;
8182
import com.cloud.vm.NicVO;
8283
import com.cloud.vm.VMInstanceVO;
8384
import com.cloud.vm.VirtualMachine;
85+
import com.cloud.vm.dao.DomainRouterDao;
8486
import com.cloud.vm.dao.NicDao;
8587
import com.cloud.vm.dao.VMInstanceDao;
8688
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
@@ -118,6 +120,8 @@ public class NetworkModelImplTest {
118120
@Mock
119121
private NicDao nicDao;
120122
@Mock
123+
private DomainRouterDao routerDao;
124+
@Mock
121125
private ServiceOfferingDao serviceOfferingDao;
122126
@Mock
123127
private ConfigurationManager configMgr;
@@ -596,12 +600,12 @@ public void getNetworkRate_routerPublicWithGuestSibling_returnsGuestNetworkOffer
596600
VMInstanceVO vm = mock(VMInstanceVO.class);
597601
when(vm.getType()).thenReturn(VirtualMachine.Type.DomainRouter);
598602
when(vmInstanceDao.findById(vmId)).thenReturn(vm);
603+
when(routerDao.findById(vmId)).thenReturn(null);
599604
NicVO guestNic = mock(NicVO.class);
600605
when(guestNic.getNetworkId()).thenReturn(guestNetworkId);
601606
when(nicDao.listByVmId(vmId)).thenReturn(List.of(guestNic));
602607
NetworkVO guestNetwork = mock(NetworkVO.class);
603608
when(guestNetwork.getTrafficType()).thenReturn(TrafficType.Guest);
604-
when(guestNetwork.getVpcId()).thenReturn(null);
605609
when(guestNetwork.getNetworkOfferingId()).thenReturn(guestOfferingId);
606610
when(_networksDao.findById(guestNetworkId)).thenReturn(guestNetwork);
607611
when(configMgr.getNetworkOfferingNetworkRate(guestOfferingId, dataCenterId)).thenReturn(80);
@@ -616,27 +620,45 @@ public void getNetworkRate_routerPublicWithoutGuestSibling_fallsBackToNetworkOff
616620
VMInstanceVO vm = mock(VMInstanceVO.class);
617621
when(vm.getType()).thenReturn(VirtualMachine.Type.DomainRouter);
618622
when(vmInstanceDao.findById(vmId)).thenReturn(vm);
623+
when(routerDao.findById(vmId)).thenReturn(null);
619624
when(nicDao.listByVmId(vmId)).thenReturn(Collections.emptyList());
620625
mockNetworkOffering(networkOfferingId);
621626
when(configMgr.getNetworkOfferingNetworkRate(networkOfferingId, dataCenterId)).thenReturn(33);
622627

623628
assertEquals(Integer.valueOf(33), networkModel.getNetworkRate(networkId, vmId));
624629
}
625630

631+
@Test
632+
public void getNetworkRate_routerPublicWithVpcRouter_returnsVpcOfferingRateWithoutNicLookup() {
633+
long networkId = 1L, vmId = 12L, dataCenterId = 2L, vpcId = 7L, vpcOfferingId = 70L;
634+
mockNetwork(networkId, 99L, dataCenterId, TrafficType.Public);
635+
VMInstanceVO vm = mock(VMInstanceVO.class);
636+
when(vm.getType()).thenReturn(VirtualMachine.Type.DomainRouter);
637+
when(vmInstanceDao.findById(vmId)).thenReturn(vm);
638+
DomainRouterVO router = mock(DomainRouterVO.class);
639+
when(router.getVpcId()).thenReturn(vpcId);
640+
when(routerDao.findById(vmId)).thenReturn(router);
641+
VpcVO vpc = mock(VpcVO.class);
642+
when(vpc.getVpcOfferingId()).thenReturn(vpcOfferingId);
643+
when(vpcDao.findById(vpcId)).thenReturn(vpc);
644+
when(configMgr.getVpcOfferingNetworkRate(vpcOfferingId, dataCenterId)).thenReturn(10);
645+
646+
// Resolved purely from the router's own vpc_id - the guest NIC does not need to exist
647+
// in the nics table yet, matching the state during initial VR deployment.
648+
assertEquals(Integer.valueOf(10), networkModel.getNetworkRate(networkId, vmId));
649+
Mockito.verify(nicDao, Mockito.never()).listByVmId(Mockito.anyLong());
650+
}
651+
626652
@Test
627653
public void getNetworkRate_routerPublicWithVpcGuestSibling_returnsVpcOfferingRate() {
628-
long networkId = 1L, vmId = 12L, dataCenterId = 2L, guestNetworkId = 5L, vpcId = 7L, vpcOfferingId = 70L;
654+
long networkId = 1L, vmId = 12L, dataCenterId = 2L, vpcId = 7L, vpcOfferingId = 70L;
629655
mockNetwork(networkId, 99L, dataCenterId, TrafficType.Public);
630656
VMInstanceVO vm = mock(VMInstanceVO.class);
631657
when(vm.getType()).thenReturn(VirtualMachine.Type.DomainRouter);
632658
when(vmInstanceDao.findById(vmId)).thenReturn(vm);
633-
NicVO guestNic = mock(NicVO.class);
634-
when(guestNic.getNetworkId()).thenReturn(guestNetworkId);
635-
when(nicDao.listByVmId(vmId)).thenReturn(List.of(guestNic));
636-
NetworkVO guestNetwork = mock(NetworkVO.class);
637-
when(guestNetwork.getTrafficType()).thenReturn(TrafficType.Guest);
638-
when(guestNetwork.getVpcId()).thenReturn(vpcId);
639-
when(_networksDao.findById(guestNetworkId)).thenReturn(guestNetwork);
659+
DomainRouterVO router = mock(DomainRouterVO.class);
660+
when(router.getVpcId()).thenReturn(vpcId);
661+
when(routerDao.findById(vmId)).thenReturn(router);
640662
VpcVO vpc = mock(VpcVO.class);
641663
when(vpc.getVpcOfferingId()).thenReturn(vpcOfferingId);
642664
when(vpcDao.findById(vpcId)).thenReturn(vpc);
@@ -653,15 +675,17 @@ public void getNetworkRate_routerPublicWithVpcGuestSiblingButMissingVpc_fallsBac
653675
VMInstanceVO vm = mock(VMInstanceVO.class);
654676
when(vm.getType()).thenReturn(VirtualMachine.Type.DomainRouter);
655677
when(vmInstanceDao.findById(vmId)).thenReturn(vm);
678+
DomainRouterVO router = mock(DomainRouterVO.class);
679+
when(router.getVpcId()).thenReturn(vpcId);
680+
when(routerDao.findById(vmId)).thenReturn(router);
681+
when(vpcDao.findById(vpcId)).thenReturn(null);
656682
NicVO guestNic = mock(NicVO.class);
657683
when(guestNic.getNetworkId()).thenReturn(guestNetworkId);
658684
when(nicDao.listByVmId(vmId)).thenReturn(List.of(guestNic));
659685
NetworkVO guestNetwork = mock(NetworkVO.class);
660686
when(guestNetwork.getTrafficType()).thenReturn(TrafficType.Guest);
661-
when(guestNetwork.getVpcId()).thenReturn(vpcId);
662687
when(guestNetwork.getNetworkOfferingId()).thenReturn(guestOfferingId);
663688
when(_networksDao.findById(guestNetworkId)).thenReturn(guestNetwork);
664-
when(vpcDao.findById(vpcId)).thenReturn(null);
665689
when(configMgr.getNetworkOfferingNetworkRate(guestOfferingId, dataCenterId)).thenReturn(90);
666690

667691
assertEquals(Integer.valueOf(90), networkModel.getNetworkRate(networkId, vmId));

‎ui/src/components/view/DetailsTab.vue‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,9 @@
150150
<div v-else-if="item === 'networkrate'">
151151
{{ String(dataResource[item]) === '-1' ? $t('label.unlimited') : dataResource[item] }}
152152
</div>
153+
<div v-else-if="item === 'publicnetworkrate'">
154+
{{ ['-1', '0'].includes(String(dataResource[item])) ? $t('label.unlimited') : dataResource[item] }}
155+
</div>
153156
<div v-else>{{ dataResource[item] }}</div>
154157
</div>
155158
</a-list-item>

‎ui/src/components/view/ListView.vue‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,9 @@
427427
<template v-if="column.key === 'networkrate'">
428428
<span>{{ String(text) === '-1' ? $t('label.unlimited') : text }}</span>
429429
</template>
430+
<template v-if="column.key === 'publicnetworkrate'">
431+
<span>{{ ['-1', '0'].includes(String(text)) ? $t('label.unlimited') : text }}</span>
432+
</template>
430433
<template v-if="column.key === 'physicalnetworkname'">
431434
<router-link :to="{ path: '/physicalnetwork/' + record.physicalnetworkid }">{{ text }}</router-link>
432435
</template>

0 commit comments

Comments
 (0)