Closed kevin-zhonghao closed 3 years ago
@kevin-zhonghao Do we have a way to verify it locally and need this fix to be verified in the Medina cluster?
@kevin-zhonghao Do we have a way to verify it locally and need this fix to be verified in the Medina cluster?
As discussed with James, we can verify locally before DPM. For the scenario #5, since DPM's new code is not PR yet, so we can not verify end-to-end for scenario #5.
@kevin-zhonghao Do we have a way to verify it locally and need this fix to be verified in the Medina cluster?
As discussed with James, we can verify locally before DPM. For the scenario #5, since DPM's new code is not PR yet, so we can not verify end-to-end for scenario #5.
Thanks. In this case, let us keep this PR open until the E2E scenario is verified, in case we need further change. @kevin-zhonghao @cj-chung
@kevin-zhonghao Do we have a way to verify it locally and need this fix to be verified in the Medina cluster?
As discussed with James, we can verify locally before DPM. For the scenario #5, since DPM's new code is not PR yet, so we can not verify end-to-end for scenario #5.
Thanks. In this case, let us keep this PR open until the E2E scenario is verified, in case we need further change. @kevin-zhonghao @cj-chung
Agree!
Merging #624 (4074927) into master (620df1a) will increase coverage by
0.19%
. The diff coverage is57.89%
.
@@ Coverage Diff @@
## master #624 +/- ##
============================================
+ Coverage 31.90% 32.09% +0.19%
- Complexity 1249 1259 +10
============================================
Files 525 525
Lines 13490 13511 +21
Branches 1669 1673 +4
============================================
+ Hits 4304 4337 +33
+ Misses 8598 8586 -12
Partials 588 588
Impacted Files | Coverage Δ | |
---|---|---|
...r/route/service/Impl/NeutronRouterServiceImpl.java | 41.09% <54.83%> (+3.42%) |
:arrow_up: |
...rewei/alcor/route/controller/RouterController.java | 48.24% <66.66%> (ø) |
|
...lcor/route/controller/NeutronRouterController.java | 59.52% <75.00%> (ø) |
|
...alcor/elasticipmanager/dao/ElasticIpAllocator.java | 63.81% <0.00%> (+0.27%) |
:arrow_up: |
...lcor/portmanager/processor/DataPlaneProcessor.java | 70.27% <0.00%> (+0.54%) |
:arrow_up: |
...rewei/alcor/portmanager/processor/PortContext.java | 71.42% <0.00%> (+3.89%) |
:arrow_up: |
...i/alcor/portmanager/processor/RouterProcessor.java | 85.29% <0.00%> (+5.88%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 620df1a...4074927. Read the comment docs.
@kevin-zhonghao We have merged the routerId part in this PR in my previous PR#627.
Can you close this PR and create a new PR for constructInternalSubnetRoutingTables
part only in this PR?
@kevin-zhonghao We have merged the routerId part in this PR in my previous PR#627. Can you close this PR and create a new PR for
constructInternalSubnetRoutingTables
part only in this PR?
Have done! Thanks for reminder James.
It seems like update routing rules is not correct. Instead update existing routing rule it insert a new one.
It seems like update routing rules is not correct. Instead update existing routing rule it insert a new one.
The current logic is that whether the routing rules are updated depends on whether the first half of the Destination field is the same. If they are the same, it should be attributed to the update operation. If it is different, it will create a new one. Could you tell more about test payload?
Did not modify destination data and only modify nexthop data. so the first half of the Destination field is the same. Code logic same like if destination and nexthop same, isExit will be true and not update route rule, if it false, Instead of update exist one in routeEntites list it create new routerEntry and add to routeEntities List.
Did the following test on 169. And saw the log from routemanager
rally --config-file /root/rally.conf task start ym_create_and_delete_subnets.json
2021-06-29 18:16:14.473 DEBUG 1 --- [nio-8080-exec-7] o.a.coyote.http11.Http11InputBuffer : Received [DELETE /project/4ea3496b3119457ba318b917b118de66/subnets/a68d33d9-3e44-43bc-a9fe-4f6e8cf0d8dc/routetable HTTP/1.1 Accept: / User-Agent: Java/11.0.8 Host: routemanager-service.default.svc.cluster.local:9003 Connection: keep-alive
] 2021-06-29 18:16:14.474 DEBUG 1 --- [nio-8080-exec-7] o.apache.catalina.valves.RemoteIpValve : Incoming request /project/4ea3496b3119457ba318b917b118de66/subnets/a68d33d9-3e44-43bc-a9fe-4f6e8cf0d8dc/routetable with originalRemoteAddr [172.16.247.243], originalRemoteHost=[172.16.247.243], originalSecure=[false], originalScheme=[http], originalServerName=[routemanager-service.default.svc.cluster.local], originalServerPort=[9003] will be seen as newRemoteAddr=[172.16.247.243], newRemoteHost=[172.16.247.243], newSecure=[false], newScheme=[http], newServerName=[routemanager-service.default.svc.cluster.local], newServerPort=[9003] 2021-06-29 18:16:14.474 DEBUG 1 --- [nio-8080-exec-7] o.a.c.authenticator.AuthenticatorBase : Security checking request DELETE /project/4ea3496b3119457ba318b917b118de66/subnets/a68d33d9-3e44-43bc-a9fe-4f6e8cf0d8dc/routetable 2021-06-29 18:16:14.474 DEBUG 1 --- [nio-8080-exec-7] org.apache.catalina.realm.RealmBase : No applicable constraints defined 2021-06-29 18:16:14.474 DEBUG 1 --- [nio-8080-exec-7] o.a.c.authenticator.AuthenticatorBase : Not subject to any constraint 2021-06-29 18:16:14.475 DEBUG 1 --- [nio-8080-exec-7] c.f.alcor.common.stats.StatisticsAspect : Calculating duration of com.futurewei.alcor.route.controller.RouterController.deleteSubnetRouteTable()... 2021-06-29 18:16:14.475 DEBUG 1 --- [nio-8080-exec-7] c.f.alcor.common.stats.StatisticsAspect : Calculating duration of com.futurewei.alcor.route.dao.RouteTableRepository.findAllItems()... 2021-06-29 18:16:14.478 INFO 1 --- [nio-8080-exec-7] c.f.alcor.common.stats.StatisticsAspect : com.futurewei.alcor.route.dao.RouteTableRepository.findAllItems() startTime: 27368378977816212ns, endTime: 27368378981184692ns, duration: 3ms 2021-06-29 18:16:14.478 DEBUG 1 --- [nio-8080-exec-7] c.f.alcor.common.stats.StatisticsAspect : Calculating duration of com.futurewei.alcor.route.service.Impl.RouteTableDatabaseServiceImpl.addRouteTable()... 2021-06-29 18:16:14.479 DEBUG 1 --- [nio-8080-exec-7] c.f.alcor.common.stats.StatisticsAspect : Calculating duration of com.futurewei.alcor.route.dao.RouteTableRepository.addItem()... 2021-06-29 18:16:14.479 WARN 1 --- [nio-8080-exec-7] global : IgniteTransaction start error:A transaction has already been started by the current thread. 2021-06-29 18:16:14.481 DEBUG 1 --- [nio-8080-exec-7] o.a.c.loader.WebappClassLoaderBase : findClass(jdk.internal.reflect.GeneratedMethodAccessor58) 2021-06-29 18:16:14.481 DEBUG 1 --- [nio-8080-exec-7] o.a.c.loader.WebappClassLoaderBase : --> Returning ClassNotFoundException 2021-06-29 18:16:14.482 ERROR 1 --- [nio-8080-exec-7] o.a.c.c.C.[.[.[/].[dispatcherServlet] : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed; nested exception is com.futurewei.alcor.common.exception.DatabasePersistenceException] with root cause
com.futurewei.alcor.common.exception.DatabasePersistenceException: null
at com.futurewei.alcor.route.service.Impl.RouteTableDatabaseServiceImpl.addRouteTable(RouteTableDatabaseServiceImpl.java:67) ~[classes!/:0.1.0-SNAPSHOT]
at com.futurewei.alcor.route.service.Impl.RouteTableDatabaseServiceImpl$$FastClassBySpringCGLIB$$e253d8db.invoke(
some comments need to make change.
@kevin-zhonghao @cj-chung It seems that we are pretty close on this one. Could we try to merge it soon?
some comments need to make change.
@kevin-zhonghao @cj-chung It seems that we are pretty close on this one. Could we try to merge it soon?
Sure, working on it with James.
@kevin-zhonghao In RouteController /project/{projectid}/subnets/{subnetid}/routetable api, constructInternalRouterInfo function first parameter routeid is not correct, could you fix it? InternalRouterInfo internalRouterInfo = this.neutronRouterService.constructInternalRouterInfo(null, internalSubnetRoutingTables)
@kevin-zhonghao In RouteController /project/{projectid}/subnets/{subnetid}/routetable api, constructInternalRouterInfo function first parameter routeid is not correct, could you fix it? InternalRouterInfo internalRouterInfo = this.neutronRouterService.constructInternalRouterInfo(null, internalSubnetRoutingTables)
Sure, already fix
Description: DPM can receive networkConfig from PM, but the routerId inside of internalRouterInfo is null.
Solution: