[Microservice] Route Manager Supports VPC Routing #380

kevin-zhonghao · 2020-09-08T23:18:38Z

Implement VPC Router in Route Manager:

CRUD APIs for VPC router
CRUD APIs for VPC route table
CRUD APIs for Subnet route table

…thController into docs/design

…o new_master

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

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

cj-chung · 2020-09-14T15:30:50Z

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

+        List<RouteTable> vpcRouteTable = router.getVpcRouteTable();
+        if (vpcRouteTable == null || vpcRouteTable.size() == 0) {
+            return null;
+        }
+        for (RouteTable routeTable : vpcRouteTable) {
+            String routeTableType = routeTable.getRouteTableType().getRouteTableType();
+            if (RouteTableType.PRIVATE_SUBNET.getRouteTableType().equals(routeTableType) || RouteTableType.PUBLIC_SUBNET.getRouteTableType().equals(routeTableType)) {
+                throw new VpcRouterContainsSubnetRoutingTables();
+            }
+        }


The logic here is not correct! We shouldn't block the updating for VPC default routing table when there are subnets using it. So, I think we don't need to check 'if the VPC router contains subnet routing table' here.

Instead, we need to check if there is any subnet exists in the VPC. If VPC contains subnet, we cannot delete VPC router and VPC default routing table.

Yeah you are right, I'll modify the checking process

cj-chung · 2020-09-14T15:39:20Z

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

+        // create a VPC routing table and pump-in the VPC default routing rules
+        RouteEntry routeEntry = new RouteEntry(projectId, routeEntryId, "default_vpc_routeEntry", "", null, RouteConstant.DEFAULT_TARGET, RouteConstant.DEFAULT_PRIORITY, routeTableId, null);
+        routeEntities.add(routeEntry);
+        this.routeEntryDatabaseService.addRouteEntry(routeEntry);
+
+        RouteTable routeTable = new RouteTable(projectId, routeTableId, "default_vpc_routeTable", "", routeEntities, RouteTableType.VPC, owner);
+        vpcRouteTables.add(routeTable);
+        this.routeTableDatabaseService.addRouteTable(routeTable);
+
+        vpcRouteTables.add(routeTable);
+        router.setVpcRouteTable(vpcRouteTables);
+        this.routerDatabaseService.addRouter(router);


These lines of code same as the lines in the getOrCreateVpcRouter function. Since one VPC has only one router and default routing table. Should we merge createDefaultVpcRouteTable into getOrCreateVpcRouter ?

Although some codes here are same as createDefaultVpcRouter, it is not convenient to merge them. They are used for different method and the parameters passed in are dfferent, either

cj-chung · 2020-09-14T15:42:34Z

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

+        List<RouteTable> vpcRouteTables = router.getVpcRouteTable();
+        for (RouteTable vpcRouteTable : vpcRouteTables) {
+            String routeTableType = vpcRouteTable.getRouteTableType().getRouteTableType();
+            if (RouteTableType.VPC.getRouteTableType().equals(routeTableType)) {
+                routeTable = vpcRouteTable;
+                vpcRouteTables.remove(vpcRouteTable);
+                break;
+            }
+        }


VPC default route table has only one for each VPC. We shouldn't look-up all routing tables to get it.

Already add a field "vpcDefaultRouteTableId" in Router to map vpc default route table

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

cj-chung · 2020-09-14T15:48:56Z

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

+            return new ArrayList<RouteTable>();
+        }
+        return router.getVpcRouteTable();
+    }


I am not quite understand the logic here. Why you return new ArrayList<RouteTable>() when you cannot find router?

I changed here return null, is it correct?

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

cj-chung · 2020-09-21T16:08:05Z

LGTM

xieus

Look good. Left a few comments, mostly minor. Please take a look. @kevin-zhonghao

xieus · 2020-09-21T18:44:25Z

lib/src/main/java/com/futurewei/alcor/common/enumClass/RouteTableType.java

    PRIVATE_SUBNET("private_subnet"),
    VPC("vpc"),
-    NEUTRON("neutron");
+    NEUTRON_ROUTER("neutron_router"),


@kevin-zhonghao @cj-chung We might want to describe each enum value a bit more, either in the code or in the design doc.

...manager/src/main/java/com/futurewei/alcor/route/exception/ExistMultipleSubnetRouteTable.java

.../route_manager/src/main/java/com/futurewei/alcor/route/exception/ExistMultipleVpcRouter.java

services/route_manager/src/main/java/com/futurewei/alcor/route/exception/VpcContainsSubnet.java

xieus · 2020-09-21T18:54:07Z

...e_manager/src/main/java/com/futurewei/alcor/route/service/Impl/NeutronRouterServiceImpl.java

            routeTable.setId(routeTableId);
            routeTable.setRouteEntities(routeEntities);
-            routeTable.setRouteTableType(RouteTableType.NEUTRON);
+            routeTable.setRouteTableType(RouteTableType.NEUTRON_ROUTER.getRouteTableType());


Why do we need getRouteTableType()

Because I changed the RouteTableType field class type

xieus · 2020-09-21T18:54:36Z

...e_manager/src/main/java/com/futurewei/alcor/route/service/Impl/NeutronRouterServiceImpl.java

            return null;
        }
-        RouteTableType routeTableType = routeTable.getRouteTableType();
+        String routeTableType = routeTable.getRouteTableType();


Should stick to RouteTableType.

xieus · 2020-09-21T18:55:12Z

...e_manager/src/main/java/com/futurewei/alcor/route/service/Impl/NeutronRouterServiceImpl.java

        if (routeTableType == null) {
            return null;
-        } else if(routeTableType.getRouteTableType().equals("neutron")){
+        } else if(routeTableType.equals("neutron")){


Enum can do comparison easier. No need to convert it to String.

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

xieus · 2020-09-21T18:57:19Z

services/route_manager/src/test/java/com/futurewei/alcor/route/NeutronRouterTests.java

        Mockito.when(routerDatabaseService.getByRouterId(UnitTestConfig.routerId))
                .thenReturn(new Router(){{setId(UnitTestConfig.routerId);setPorts(new ArrayList<>());
-                    setNeutronRouteTable(new RouteTable(){{setRouteEntities(new ArrayList<>());setRouteTableType(RouteTableType.NEUTRON);}});}});
+                    setNeutronRouteTable(new RouteTable(){{setRouteEntities(new ArrayList<>());setRouteTableType(RouteTableType.NEUTRON_ROUTER.getRouteTableType());}});}});


Same. Should get rid of getRouteTableType() here.

xieus · 2020-09-21T18:58:10Z

web/src/main/java/com/futurewei/alcor/web/entity/route/RouteTable.java


    @JsonProperty("route_table_type")
-    private RouteTableType routeTableType;
+    private String routeTableType;


Keep RouteTableType if possible.

If I keep RouteTableType, it is difficult to mock requestBody in UTs

xieus and others added 30 commits September 30, 2019 18:24

Update design documents and table of content

d745b47

Update README.md

1fc253e

Add cloud-native key word

9b0efb6

Merge branch 'docs/design' of https://github.com/futurewei-cloud/Alio…

db0a699

…thController into docs/design

Start docs for comm and controller

424dd5b

Add design sketch

b41edb9

Design spec round 1

3e54c57

Rename fast path file and start feature comparison

d7d8ad9

Update rescue path doc [DRAFT]

4c6588e

upload rescue_path.GIF

cceaa68

update rescue path needed change table

9785656

rescue_path.GIF

3e2b63e

rescue_path.adoc

f8fc472

Design spec round 2

2480dd1

Monitoring design outline

68eda0b

Update comm docs

c1c9cc7

Update data storage design

79e91fe

Merge branch 'docs/design' of https://github.com/futurewei-cloud/Alio…

ca69892

…thController into docs/design

Update data store

8b154ab

Complete review of existing dbs

4561d1e

Update review table

ca4610e

Adjust table column width

24b4a36

Update feature comparsion table

45774da

Finalize review section in data store

18e500c

Add design diagram

cc99d14

Add a design diagram

a9c8232

Add files via upload

2823921

fix "design" session level

47b7e45

Update monitoring.adoc

8c9c004

Update monitoring.adoc

4180450

kevin-zhonghao added 7 commits August 18, 2020 10:55

Merge branch 'master' of https://github.com/futurewei-cloud/alcor int…

a393405

…o new_master

Merge branch 'master' of https://github.com/futurewei-cloud/alcor int…

eabd200

…o new_master

Merge branch 'master' of https://github.com/futurewei-cloud/alcor int…

b3eec65

…o new_master

Merge branch 'master' of https://github.com/futurewei-cloud/alcor int…

88b02b7

…o new_master

Merge branch 'master' of https://github.com/futurewei-cloud/alcor int…

4bce0d7

…o new_master

Merge branch 'master' of https://github.com/futurewei-cloud/alcor int…

dee095b

…o new_master

update

dc1aced

xieus requested review from cj-chung and xieus September 8, 2020 23:19

xieus assigned kevin-zhonghao Sep 8, 2020

xieus added enhancement New feature or request feature feature development labels Sep 8, 2020

xieus added this to the Version 0.9.2020.09.30 milestone Sep 8, 2020

update

3e45fc5

cj-chung reviewed Sep 11, 2020

View reviewed changes

services/route_manager/src/main/java/com/futurewei/alcor/route/controller/RouterController.java Outdated Show resolved Hide resolved

cj-chung reviewed Sep 11, 2020

View reviewed changes

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

cj-chung suggested changes Sep 11, 2020

View reviewed changes

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

cj-chung suggested changes Sep 14, 2020

View reviewed changes

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

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

kevin-zhonghao added 4 commits September 14, 2020 16:44

UTs

312577a

update

a347ef2

update

7974891

update

a5d875c

cj-chung approved these changes Sep 21, 2020

View reviewed changes

xieus changed the title ~~Feature/Vpc_Route_APIs~~ [Microservice] Route Manager Supports VPC Routing Sep 21, 2020

xieus reviewed Sep 21, 2020

View reviewed changes

update

a9970dc

xieus approved these changes Sep 21, 2020

View reviewed changes

xieus merged commit 1f9d405 into futurewei-cloud:master Sep 21, 2020

[Microservice] Route Manager Supports VPC Routing #380

[Microservice] Route Manager Supports VPC Routing #380

Uh oh!

Conversation

kevin-zhonghao commented Sep 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cj-chung commented Sep 21, 2020

Uh oh!

xieus left a comment

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kevin-zhonghao commented Sep 8, 2020 •

edited

Loading