fix ip VIC plugin throws a parsing error when customer tries to speci… #583
Conversation
38eba89 to
24c5d2c
Compare
|
Adding @javierfz1980 and @lmalvins as I'm not a familiar enough with our UI code to properly review this. |
| @@ -0,0 +1,13461 @@ | |||
| { | |||
There was a problem hiding this comment.
This seems unrelated to the change being made. Is this supposed to be included in this PR?
There was a problem hiding this comment.
yes, not related with this issue, but necessary
| @@ -70,6 +70,7 @@ export class NetworksComponent implements OnInit { | |||
| Validators.pattern(ipPattern) | |||
| ]], | |||
| clientNetworkRouting: [{ value: '', disabled: true }, [ | |||
| Validators.required, | |||
There was a problem hiding this comment.
It looks like this may make the field required. I'm not sure that's the correct behavior. It seems like we want to require that either both client-network-routing and client-network-gateway are supplied or both are blank.
There was a problem hiding this comment.
Steven had checked the code in backend. This field is required.
There was a problem hiding this comment.
Zach, for the client-network-gateway is it also set for the default route which vCH assigned IP range belongs to? or just for the optional IP range?
There was a problem hiding this comment.
| <span class="tooltip-content" *ngIf="form.get('clientNetworkGateway').hasError('pattern')"> | ||
| Gateway address is not valid | ||
| <span class="tooltip-content" *ngIf="form.get('clientNetworkRouting').hasError('pattern')"> | ||
| IP addresses are not valid,please input one or more routing destinations in a comma separated list, e.g. 10.1.0.0/16,10.2.0.0/16:10.0.0.1 |
There was a problem hiding this comment.
IP addresses are not valid,please input ...
There's a space missing after the comma ("valid, please").
e.g. 10.1.0.0/16,10.2.0.0/16:10.0.0.1
This content seems to be for the routing destinations field. Maybe it should exclude the ":10.0.0.1" from the example, since that goes in the other field?
| </span> | ||
|
|
||
| <span class="tooltip-content" *ngIf="form.get('clientNetworkGateway').hasError('pattern')"> | ||
| Gateway address is not valid, please input ip address with format *.*.*.*. e.g. 10.1.119.23 |
There was a problem hiding this comment.
ip address
It seems like we capitalize "IP" elsewhere in the UI.
with format .... e.g. 10.1.119.23
This may just be personal preference, but I find the "..." hard to read. I think most users should understand how to format an IP address, so the example is probably sufficient.
2176327 to
672fd6a
Compare
|
@zjs , now I modify the destination routing to be optional. But if users input the static ip and gateway without destination routing, backend will return "parsing vch body from "" failed, because json: cannot unmarshal object into Go struct field Network.static of type models.CIDR" error message. I tried to not pass destination routing in the payload, also tried to set it empty. All returns same error. I believe logic should be modified in the backend to support it. |
fix ip VIC plugin throws a parsing error when customer tries to specify only the gateway for management network.
When you provide a gateway for the management network, it's mandatory to provide at least one routing destination. UI Plugin doesn't reflect that. It has two separate fields for Gateway and Routing destinations. If the customer doesn't provide both the fields properly, plugin throws a weird parsing error [Refer to the screenshot].
Fix:
1 Modify UI to make routing destination required.
2 Modify the components destination input and ip address to one input.
Signed-off-by: Meina Zhou <meinaz@vmware.com>
Signed-off-by: Meina Zhou <meinaz@vmware.com>
672fd6a to
222ceac
Compare
…re#583) fix ip VIC plugin throws a parsing error when customer tries to specify only the gateway for management network. When you provide a gateway for the management network, it's mandatory to provide at least one routing destination. UI Plugin doesn't reflect that. It has two separate fields for Gateway and Routing destinations. If the customer doesn't provide both the fields properly, plugin throws a weird parsing error [Refer to the screenshot]. Fix: 1 Modify UI to make routing destination required. 2 Modify the components destination input and ip address to one input. Signed-off-by: Meina Zhou <meinaz@vmware.com> Signed-off-by: Meina Zhou <meinaz@vmware.com> (cherry picked from commit 3817223)
#595) fix ip VIC plugin throws a parsing error when customer tries to specify only the gateway for management network. When you provide a gateway for the management network, it's mandatory to provide at least one routing destination. UI Plugin doesn't reflect that. It has two separate fields for Gateway and Routing destinations. If the customer doesn't provide both the fields properly, plugin throws a weird parsing error [Refer to the screenshot]. Fix: 1 Modify UI to make routing destination required. 2 Modify the components destination input and ip address to one input. Signed-off-by: Meina Zhou <meinaz@vmware.com> Signed-off-by: Meina Zhou <meinaz@vmware.com> (cherry picked from commit 3817223)
issue description:
When you provide a gateway for the management network, it's mandatory to provide at least one routing destination. UI Plugin doesn't reflect that. It has two separate fields for Gateway and Routing destinations. If the customer doesn't provide both the fields properly, plugin throws a weird parsing error [Refer to the screenshot].
Fix:
1 Modify UI to make routing destination required.
2 Modify the components destination input and ip address to one input.
Signed-off-by: Meina Zhou meinaz@vmware.com
Fixes #
PR acceptance checklist:
[ ] All unit tests pass
[ ] All e2e tests pass
[ ] Unit test(s) included*
[ ] e2e test(s) included*
[ ] Screenshot attached and UX approved*
*if applicable, add n/a if not