[RM] Separate getOrCreateVPCRouter into GET and POST two APIs #655

DavidLiu506 · 2021-06-29T19:12:20Z

Router controller:

getOrCreateVPCRouter
-> rename as 'getVPCRoutercreate', and delete 'create' operation related part
-> Add a new API 'createVPCRouter'

getOrCreateSubnetRouteTable
-> rename as 'getSubnetRouteTable', and delete 'create' operation related part

createNeutronSubnetRouteTable
-> rename as 'createSubnetRouteTable'

getOrCreateVpcRouteTable
-> rename as 'getVpcRouteTable', and delete 'create' operation related part

-> Add a new API 'createVpcRouteTable'

services/route_manager/src/main/java/com/futurewei/alcor/route/controller/RouterController.java

cj-chung · 2021-06-30T16:12:03Z

services/route_manager/src/main/java/com/futurewei/alcor/route/controller/RouterController.java

    public RouteTablesWebJson getVpcRouteTables(@PathVariable String projectid, @PathVariable String vpcid) throws Exception {

-        List<RouteTable> routetables = new ArrayList<>();
+        List<RouteTable> routetables = null;


@DavidLiu506 If you set this as null and this function cannot find vpcroutetables, line 299 in this code will get NPE (null point exception).

Also, please uncomment line 309 in this file. We need to send internalRouterInfo to DPM.

line send internalRouterInfo to DPM uncomment.

updateVpcRouteTable in RouterController will not get NPE because updateVpcRouteTable in RouterServiceImpl already handle it.

@DavidLiu506 I didn't see you have handled if this function return null in updateVpcRouteTable:

List<RouteTable> vpcRouteTables = router.getVpcRouteTables(); for (RouteTable vpcRouteTable : vpcRouteTables) { String routeTableType = vpcRouteTable.getRouteTableType(); if (RouteTableType.VPC.getRouteTableType().equals(routeTableType)) { return vpcRouteTable; } }

Sorry, you are right. Fixed.

cj-chung · 2021-06-30T16:14:33Z

services/route_manager/src/main/java/com/futurewei/alcor/route/controller/RouterController.java

            value = {"/project/{projectid}/subnets/{subnetid}/routetable"})
    @DurationStatistics
-    public RouteTableWebJson createNeutronSubnetRouteTable(@PathVariable String projectid, @PathVariable String subnetid, @RequestBody RouteTableWebJson resource) throws Exception {
+    public RouteTableWebJson createSubnetRouteTable(@PathVariable String projectid, @PathVariable String subnetid, @RequestBody RouteTableWebJson resource) throws Exception {


@kevin-zhonghao Did you also modify this function? Is that ok to rename this function?
@DavidLiu506 Please check PR#657

...es/route_manager/src/main/java/com/futurewei/alcor/route/service/Impl/RouterServiceImpl.java

cj-chung · 2021-06-30T16:26:52Z

...es/route_manager/src/main/java/com/futurewei/alcor/route/service/Impl/RouterServiceImpl.java


    @Override
-    public RouteTable getSubnetRouteTable(String projectId, String subnetId) throws CacheException, OwnMultipleSubnetRouteTablesException, DatabasePersistenceException, ResourceNotFoundException, ResourcePersistenceException, CanNotFindSubnet, OwnMultipleVpcRouterException, CanNotFindVpc {
+    public RouteTable getSubnetRouteTable(String projectId, String subnetId) throws CacheException, CanNotFindRouteTableByOwner, OwnMultipleSubnetRouteTablesException {


@kevin-zhonghao Did you also update this function? Please confirm!

...es/route_manager/src/main/java/com/futurewei/alcor/route/service/Impl/RouterServiceImpl.java

cj-chung · 2021-06-30T16:39:32Z

services/route_manager/src/main/java/com/futurewei/alcor/route/controller/RouterController.java

            value = {"/project/{projectid}/subnets/{subnetid}/routetable"})
    @DurationStatistics
-    public RouteTableWebJson getOrCreateSubnetRouteTable(@PathVariable String projectid, @PathVariable String subnetid) throws Exception {
+    public RouteTableWebJson getSubnetRouteTable(@PathVariable String projectid, @PathVariable String subnetid) throws Exception {


@kevin-zhonghao Please check if you also modify this function in your PR.
@DavidLiu506 Please check PR#657

cj-chung · 2021-07-06T20:50:28Z

services/route_manager/src/main/java/com/futurewei/alcor/route/controller/RouterController.java

+            if (routetables == null)
+            {
+                throw new RouterUnavailable();
+            }
+


@DavidLiu506 Is this a RouterUnavailable or RouteTableUnavailable?

cj-chung · 2021-07-06T21:57:24Z

services/route_manager/src/main/java/com/futurewei/alcor/route/controller/RouterController.java

            routes.add(defaultRoute);

-            routeTable = this.routerService.createNeutronSubnetRouteTable(projectid, subnetid, resource, routes);
+            routeTable = this.routerService.createSubnetRouteTable(projectid, subnetid, resource, routes);


@DavidLiu506 We need to remove line#454~#456 defaultRoute setting.

cj-chung

LGTM

kevin-zhonghao · 2021-07-07T18:39:53Z

LGTM.

PS: Have reviewed PR #655 and #655 has already covered all change in PR #657.
So close the PR #657.

Update router manager api

04cbb1e

DavidLiu506 requested review from cj-chung and xieus June 29, 2021 19:18

xieus assigned DavidLiu506 Jun 29, 2021

xieus added the bug Something isn't working label Jun 29, 2021

xieus added this to the Version 0.17.2021.07.30 milestone Jun 29, 2021

cj-chung changed the title ~~Update router manager api~~ [RM] Separate getOrCreateVPCRouter into GET and POST two APIs Jun 29, 2021

Update code

4c100e5

This was linked to issues Jun 30, 2021

[RM] getOrCreateVpcRouter in RouterController.java need a flag to indicate if it requires to create a router #646

Closed

[RM] Separate getOrCreateSubnetRouteTable into two APIs and consolidate POST with createNeutronSubnetRouteTable #654

Closed

cj-chung suggested changes Jun 30, 2021

View reviewed changes

cj-chung reviewed Jun 30, 2021

View reviewed changes

...es/route_manager/src/main/java/com/futurewei/alcor/route/service/Impl/RouterServiceImpl.java Show resolved Hide resolved

cj-chung reviewed Jun 30, 2021

View reviewed changes

DavidLiu506 added 6 commits June 30, 2021 16:42

Update code

e38bcf9

Update router manager

067a988

Update router manager

dd5dafd

Merge router manager service api code

aa76562

Fix getVpcRouteTables

8ae37db

Fix getVpcRouteTable

f0462ae

cj-chung reviewed Jul 6, 2021

View reviewed changes

Update RouterController

976883f

cj-chung reviewed Jul 6, 2021

View reviewed changes

Update code

ef7a2d2

cj-chung approved these changes Jul 7, 2021

View reviewed changes

cj-chung mentioned this pull request Jul 7, 2021

[RM] Fix Issue#647 and Issue#654 #657

Closed

xieus merged commit 799ee03 into futurewei-cloud:master Jul 7, 2021

[RM] Separate getOrCreateVPCRouter into GET and POST two APIs #655

[RM] Separate getOrCreateVPCRouter into GET and POST two APIs #655

Uh oh!

Conversation

DavidLiu506 commented Jun 29, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cj-chung Jun 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cj-chung left a comment

Choose a reason for hiding this comment

Uh oh!

kevin-zhonghao commented Jul 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cj-chung Jun 30, 2021 •

edited

Loading