PUT/DELETE NC and subnet join related changes for RNC via NMA - #4597
PUT/DELETE NC and subnet join related changes for RNC via NMA#4597Saksham Mittal (smittal22) wants to merge 10 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
This PR starts wiring “RNC via NMA” support into CNS publish/unpublish flows by threading a UseRNCPublisher flag through request contracts and the wireserver proxy, and introducing a subnet-join step (in addition to vnet join) when that flag is enabled.
Changes:
- Add
UseRNCPublisherto NMAgent publish/unpublish request bodies and thread it through CNS publish/unpublish handler logic. - Extend wireserver proxy operations to optionally set
useLegacyChannel=falseand add a newJoinSubnetcall for RNC flows. - Add CNS response code
SubnetJoinFailedand in-memory subnet-joined state tracking.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| nmagent/requests.go | Adds UseRNCPublisher to NC put/delete request JSON contracts. |
| cns/wireserver/proxy.go | Updates Join/Publish/Unpublish to optionally use non-legacy channel; adds JoinSubnet request. |
| cns/types/codes.go | Introduces SubnetJoinFailed response code and string mapping. |
| cns/restserver/util.go | Adds subnet joined-state helpers (isSubnetJoined, setSubnetStateJoined). |
| cns/restserver/restserver.go | Extends wireserverProxy interface and restserver state with joinedSubnets. |
| cns/restserver/const.go | Adds named-lock key for subnet joined-state. |
| cns/restserver/api.go | Implements RNC-aware JoinNetwork/JoinSubnet and passes the flag into publish/unpublish calls. |
| cns/NetworkContainerContract.go | Extends publish/unpublish request contracts to carry SubnetName (for subnet join). |
| cns/fakes/wireserverproxyfake.go | Updates fake proxy to match the expanded wireserver proxy interface. |
Comments suppressed due to low confidence (2)
cns/restserver/api.go:1006
- On subnet join error, the response Message formats the wrong error variable (err is nil here), so the client gets a misleading message.
ReturnCode: types.SubnetJoinFailed,
Message: fmt.Sprintf("failed to join subnet %s in network %s: %v", req.SubnetName, req.NetworkID, err),
},
PublishErrorStr: errSubnetJoin.Error(),
cns/restserver/api.go:1202
- This is the unpublish path but the error message says "failed to publish nc". This makes troubleshooting confusing.
Response: cns.Response{
ReturnCode: types.NetworkContainerUnpublishFailed,
Message: fmt.Sprintf("failed to publish nc %s: %v", req.NetworkContainerID, err),
},
UnpublishErrorStr: err.Error(),
a96e24a to
44f9b4f
Compare
|
/azp |
Supported commands
See additional documentation. |
|
/azp run Azure Container Networking PR |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Copilot review the PR |
Co-authored-by: smittal22 <111590532+smittal22@users.noreply.github.com>
Co-authored-by: smittal22 <111590532+smittal22@users.noreply.github.com>
Co-authored-by: smittal22 <111590532+smittal22@users.noreply.github.com>
|
There was an error handling pipeline event f4a401db-7416-4c05-a345-3484056d7a57. |
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
|
Pull request closed due to inactivity. |
|
Reopening PR |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
| GREKey uint16 `json:"greKey"` | ||
| AzID uint `json:"azID"` | ||
| AZREnabled bool `json:"azrEnabled"` | ||
| UseRNCPublisher bool `json:"useRNCPublisher"` |
| // PublishNetworkContainerRequest specifies request to publish network container via NMAgent. | ||
| type PublishNetworkContainerRequest struct { | ||
| NetworkID string | ||
| SubnetName string |
There was a problem hiding this comment.
Is this just for logging and debugging?
Reason for Change:
Issue Fixed:
Requirements:
Notes: