Update CNS to handle updated DNC API in managed mode #725
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| package dncclient | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "encoding/json" | ||
| "fmt" | ||
| "io" | ||
| "net/http" | ||
| "runtime" | ||
| "time" | ||
|
|
||
| "github.com/Azure/azure-container-networking/cns" | ||
| "github.com/Azure/azure-container-networking/cns/logger" | ||
| acn "github.com/Azure/azure-container-networking/common" | ||
| ) | ||
|
|
||
| const ( | ||
| // RegisterNodeURLFmt: /networks/{infraNetworkID}/node/{nodeID} | ||
| registerNodeURLFmt = "%s/networks/%s/node/%s%s" | ||
| // SyncNodeNetworkContainersURLFmt: /networks/{infraNetworkID}/node/{nodeID}/networkcontainers | ||
| syncNodeNetworkContainersURLFmt = "%s/networks/%s/node/%s/networkcontainers%s" | ||
|
|
||
| dncAPIVersion = "?api-version=2018-03-01" | ||
| registerNodeRetryInterval = 5 * time.Second | ||
| ) | ||
|
|
||
| // NodeRegistrationRequest - Struct to hold node registration request. | ||
| type NodeRegistrationRequest struct { | ||
| NumCores int `json:"NumCores"` | ||
| } | ||
|
|
||
| // RegisterNode registers the node with managed DNC | ||
| func RegisterNode(httpRestService cns.HTTPService, dncEP, infraVnet, nodeID string) { | ||
| logger.Printf("[dncclient] Registering node: %s with Infrastructure Network: %s dncEP: %s", nodeID, infraVnet, dncEP) | ||
|
|
||
| var ( | ||
| registerNodeURL = fmt.Sprintf(registerNodeURLFmt, dncEP, infraVnet, nodeID, dncAPIVersion) | ||
| body bytes.Buffer | ||
| httpc = acn.GetHttpClient() | ||
| ) | ||
|
|
||
| // Create a body with number of CPU cores | ||
| nodeRegistrationRequest := NodeRegistrationRequest{ | ||
| NumCores: runtime.NumCPU(), | ||
| } | ||
| json.NewEncoder(&body).Encode(nodeRegistrationRequest) | ||
|
|
||
| for { | ||
| orchestratorDetails, err := registerNode(httpc, registerNodeURL, &body) | ||
| if err != nil { | ||
| logger.Errorf("[dncclient] Failed to register node: %s with error: %+v", nodeID, err) | ||
| // todo: make this interval configurable | ||
| time.Sleep(registerNodeRetryInterval) | ||
| continue | ||
| } | ||
|
|
||
| httpRestService.SetNodeOrchestrator(&orchestratorDetails) | ||
| break | ||
| } | ||
|
|
||
| logger.Printf("[dncclient] Successfully registered node: %s", nodeID) | ||
| } | ||
|
|
||
| func registerNode(httpCl *http.Client, url string, body io.Reader) (cns.SetOrchestratorTypeRequest, error) { | ||
| var orchestratorDetails cns.SetOrchestratorTypeRequest | ||
| response, err := httpCl.Post(url, acn.JsonContent, body) | ||
| if err != nil { | ||
| return orchestratorDetails, err | ||
| } | ||
|
|
||
| defer response.Body.Close() | ||
|
|
||
| if response.StatusCode != http.StatusOK { | ||
| return orchestratorDetails, | ||
| fmt.Errorf("[dncclient] Failed to register node with http status code: %d", response.StatusCode) | ||
| } | ||
|
|
||
| _ = json.NewDecoder(response.Body).Decode(&orchestratorDetails) | ||
| return orchestratorDetails, nil | ||
| } | ||
|
|
||
| // SyncNodeNcStatus retrieves the NCs scheduled on this node by DNC | ||
| func SyncNodeNcStatus(dncEP, infraVnet, nodeID string) (cns.NodeInfoResponse, error) { | ||
| var ( | ||
| syncNodeNcStatusURL = fmt.Sprintf(syncNodeNetworkContainersURLFmt, dncEP, infraVnet, nodeID, dncAPIVersion) | ||
| nodeInfoResponse cns.NodeInfoResponse | ||
| httpc = acn.GetHttpClient() | ||
| ) | ||
|
|
||
| logger.Printf("[dncclient] SyncNodeNcStatus: Node: %s, InfraVnet: %s", nodeID, infraVnet) | ||
|
|
||
| response, err := httpc.Get(syncNodeNcStatusURL) | ||
| if err != nil { | ||
| return nodeInfoResponse, err | ||
| } | ||
|
|
||
| if response.StatusCode == http.StatusOK { | ||
| err = json.NewDecoder(response.Body).Decode(&nodeInfoResponse) | ||
| } else { | ||
| err = fmt.Errorf("%d", response.StatusCode) | ||
| } | ||
| response.Body.Close() | ||
|
|
||
| return nodeInfoResponse, err | ||
|
Contributor
There was a problem hiding this comment. prefer to return early. it's easier then to read the happy path of the code without much indentation. |
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that this is code that was moved, but perhaps it's a good chance to clean it up since it is a bit hard to follow. My main recommendation would be to extract the body of the loop to a function such as:
which then makes the loop much simpler:
To go a step further, I would consider making a proper
DNCClientstruct, which embeds reusable dependencies such as an http client. The main benefit of this is that the calling code can then use an interface and stub out the actual implementation when writing tests. That can be a later refactor though. See cnsclient in the dnc repo as an example.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I have updated the logic. I will create the DNCClient struct as a part of next PR where I'll handle the msi in the dncclient.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refactored this section in #680 , along with Ramiro's suggestion, i added ticker logic too