[DPM/ACA Schema] Add message_type and Update Router & SG Configuration #362
Conversation
| string ipv4_address = 6; | ||
| string ipv6_address = 7; | ||
| string port_host_name = 8; // for local DNS response | ||
| MessageType message_type = 4; // DELTA (default) or FULL |
There was a problem hiding this comment.
Do we differentiate message_type for DHCP, or everything is FULL for DHCP?
There was a problem hiding this comment.
I see two possible usage of message_type for DHCP:
- To update the repeated extra_dhcp_options or dns_entry_list in the future
- When the DHCP resource is out of order/error, it will look for a FULL message_type to reset
We may go with everything FULL approach for DHCP, but it will have less flexability and not consistant with other resource types.
There was a problem hiding this comment.
we need PM to provide the api then
There was a problem hiding this comment.
DHCP configuration is generated by DPM, if additional information is needed PM should provide it based on the contract between DPM<-> PM.
There was a problem hiding this comment.
@chenpiaoping what do you think about "PM should provide it based on the contract between DPM<-> PM"
schema/proto3/vpc.proto
Outdated
| @@ -23,16 +23,17 @@ option java_outer_classname = "Vpc"; | |||
|
|
|||
| import "common.proto"; | |||
|
|
|||
| message VpcConfiguration { | |||
| message VpcConfiguration { // are we ready to remove VpcState based on the current router discussion? | |||
There was a problem hiding this comment.
Is VpcConfiguration in any case used for info purpose?
There was a problem hiding this comment.
not during our discussion so far including current router design
There was a problem hiding this comment.
Okay, I could bear with it for some more time unless we complete the design for all major features. It is up to you though :-)
There was a problem hiding this comment.
I will change the comment to below so that we can bear with it for now :)
// Not used based on the current design including router E2E
// To be removed once we complete the design for all major features
message VpcConfiguration {
There was a problem hiding this comment.
when would you remove it @er1cthe0ne
There was a problem hiding this comment.
To be removed once we complete the design for all major features, on top of my head, that would include SG, EIP, SNAT.
|
@er1cthe0ne Approved the change. As this could be a breaking change as expected, maybe we could merge to master after this week's release, or you urgently need this merge? |
Agreed, we can merge it after this week's release. I am not blocked on my GRPC streaming implementation/test since I can use local change. |
Codecov Report
@@ Coverage Diff @@
## master #362 +/- ##
============================================
+ Coverage 33.98% 34.03% +0.04%
- Complexity 860 887 +27
============================================
Files 354 367 +13
Lines 8907 9165 +258
Branches 1081 1127 +46
============================================
+ Hits 3027 3119 +92
- Misses 5476 5652 +176
+ Partials 404 394 -10 Continue to review full report at Codecov.
|
er1cthe0ne
changed the title
xieus
changed the title
…ion (#362) Co-authored-by: albert <[email protected]> Co-authored-by: Liguang Xie <[email protected]>
This DPM/ACA Schema update includes the following: