From 32deebcd826e60431168c9ee53741ed5b0bca785 Mon Sep 17 00:00:00 2001 From: Saksham Mittal Date: Tue, 17 Oct 2023 12:26:38 -0700 Subject: [PATCH 01/12] change expected code to 200, since DNC returns 200 --- cns/service/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/service/main.go b/cns/service/main.go index d80b90b286..8b5c6f45e6 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -423,7 +423,7 @@ func sendRegisterNodeRequest(httpc *http.Client, httpRestService cns.HTTPService } defer response.Body.Close() - if response.StatusCode != http.StatusCreated { + if response.StatusCode != http.StatusOK { err = fmt.Errorf("[Azure CNS] Failed to register node, DNC replied with http status code %s", strconv.Itoa(response.StatusCode)) logger.Errorf(err.Error()) return errors.Wrap(err, "failed to sendRegisterNodeRequest") From 12b8dd4d2de2fe4cb8b38955cbc6e3a4fc6b8780 Mon Sep 17 00:00:00 2001 From: Saksham Mittal Date: Thu, 7 Dec 2023 10:46:36 -0800 Subject: [PATCH 02/12] Use http client interface instead of passing in a client directly --- cns/service/main.go | 12 +++++++----- common/utils.go | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/cns/service/main.go b/cns/service/main.go index 8b5c6f45e6..b8d0196d80 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -373,7 +373,7 @@ type NodeInterrogator interface { } // RegisterNode - Tries to register node with DNC when CNS is started in managed DNC mode -func registerNode(httpc *http.Client, httpRestService cns.HTTPService, dncEP, infraVnet, nodeID string, ni NodeInterrogator) error { +func registerNode(httpClient acn.HTTPClient, httpRestService cns.HTTPService, dncEP, infraVnet, nodeID string, ni NodeInterrogator) error { logger.Printf("[Azure CNS] Registering node %s with Infrastructure Network: %s PrivateEndpoint: %s", nodeID, infraVnet, dncEP) var ( @@ -400,7 +400,7 @@ func registerNode(httpc *http.Client, httpRestService cns.HTTPService, dncEP, in // CNS tries to register Node for maximum of an hour. err := retry.Do(func() error { - return sendRegisterNodeRequest(httpc, httpRestService, nodeRegisterRequest, url) + return sendRegisterNodeRequest(httpClient, httpRestService, nodeRegisterRequest, url) }, retry.Delay(acn.FiveSeconds), retry.Attempts(maxRetryNodeRegister), retry.DelayType(retry.FixedDelay)) return errors.Wrap(err, fmt.Sprintf("[Azure CNS] Failed to register node %s after maximum reties for an hour with Infrastructure Network: %s PrivateEndpoint: %s", @@ -408,7 +408,7 @@ func registerNode(httpc *http.Client, httpRestService cns.HTTPService, dncEP, in } // sendRegisterNodeRequest func helps in registering the node until there is an error. -func sendRegisterNodeRequest(httpc *http.Client, httpRestService cns.HTTPService, nodeRegisterRequest cns.NodeRegisterRequest, registerURL string) error { +func sendRegisterNodeRequest(httpClient acn.HTTPClient, httpRestService cns.HTTPService, nodeRegisterRequest cns.NodeRegisterRequest, registerURL string) error { var body bytes.Buffer err := json.NewEncoder(&body).Encode(nodeRegisterRequest) if err != nil { @@ -416,7 +416,7 @@ func sendRegisterNodeRequest(httpc *http.Client, httpRestService cns.HTTPService return errors.Wrap(retry.Unrecoverable(err), "failed to sendRegisterNodeRequest") } - response, err := httpc.Post(registerURL, "application/json", &body) + response, err := httpClient.Post(registerURL, "application/json", body.Bytes()) if err != nil { logger.Errorf("[Azure CNS] Failed to register node with retriable err: %+v", err) return errors.Wrap(err, "failed to sendRegisterNodeRequest") @@ -891,7 +891,9 @@ func main() { httpRestService.SetOption(acn.OptInfrastructureNetworkID, infravnet) httpRestService.SetOption(acn.OptNodeID, nodeID) - registerErr := registerNode(acn.GetHttpClient(), httpRestService, privateEndpoint, infravnet, nodeID, nmaClient) + standardClient := &acn.StandardHTTPClient{} + + registerErr := registerNode(standardClient, httpRestService, privateEndpoint, infravnet, nodeID, nmaClient) if registerErr != nil { logger.Errorf("[Azure CNS] Resgistering Node failed with error: %v PrivateEndpoint: %s InfrastructureNetworkID: %s NodeID: %s", registerErr, diff --git a/common/utils.go b/common/utils.go index cd6a5bffa0..ded8259ada 100644 --- a/common/utils.go +++ b/common/utils.go @@ -4,6 +4,7 @@ package common import ( + "bytes" "context" "encoding/binary" "encoding/json" @@ -78,6 +79,19 @@ type metadataWrapper struct { Metadata Metadata `json:"compute"` } +// HTTPClient interface to abstract http.Client methods +type HTTPClient interface { + Post(url string, contentType string, body []byte) (*http.Response, error) +} + +// StandardHTTPClient is a standard implementation of the HTTPClient interface +type StandardHTTPClient struct{} + +// Post is the implementation of the Post method for StandardHTTPClient +func (s *StandardHTTPClient) Post(url string, contentType string, body []byte) (*http.Response, error) { + return http.Post(url, contentType, bytes.NewBuffer(body)) +} + // Creating http client object to be reused instead of creating one every time. // This helps make use of the cached tcp connections. // Clients are safe for concurrent use by multiple goroutines. From 8e87927175f48fd3ec7f9f108d8b824cf45360ee Mon Sep 17 00:00:00 2001 From: Saksham Mittal Date: Thu, 7 Dec 2023 17:11:39 -0800 Subject: [PATCH 03/12] Write test case --- cns/service/main.go | 6 +++--- cns/service/main_test.go | 41 ++++++++++++++++++++++++++++++++++++++++ common/utils.go | 4 ++-- 3 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 cns/service/main_test.go diff --git a/cns/service/main.go b/cns/service/main.go index b8d0196d80..b113f1a4b2 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -412,13 +412,13 @@ func sendRegisterNodeRequest(httpClient acn.HTTPClient, httpRestService cns.HTTP var body bytes.Buffer err := json.NewEncoder(&body).Encode(nodeRegisterRequest) if err != nil { - log.Errorf("[Azure CNS] Failed to register node while encoding json failed with non-retriable err %v", err) + log.Errorf("[Azure CNS] Failed to register node while encoding json failed with non-retryable err %v", err) return errors.Wrap(retry.Unrecoverable(err), "failed to sendRegisterNodeRequest") } response, err := httpClient.Post(registerURL, "application/json", body.Bytes()) if err != nil { - logger.Errorf("[Azure CNS] Failed to register node with retriable err: %+v", err) + logger.Errorf("[Azure CNS] Failed to register node with retryable err: %+v", err) return errors.Wrap(err, "failed to sendRegisterNodeRequest") } defer response.Body.Close() @@ -432,7 +432,7 @@ func sendRegisterNodeRequest(httpClient acn.HTTPClient, httpRestService cns.HTTP var req cns.SetOrchestratorTypeRequest err = json.NewDecoder(response.Body).Decode(&req) if err != nil { - log.Errorf("[Azure CNS] decoding Node Resgister response json failed with err %v", err) + log.Errorf("[Azure CNS] decoding Node Register response json failed with err %v", err) return errors.Wrap(err, "failed to sendRegisterNodeRequest") } httpRestService.SetNodeOrchestrator(&req) diff --git a/cns/service/main_test.go b/cns/service/main_test.go new file mode 100644 index 0000000000..e86c09b241 --- /dev/null +++ b/cns/service/main_test.go @@ -0,0 +1,41 @@ +package main + +import ( + "bytes" + "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/fakes" + "io/ioutil" + "net/http" + "testing" +) + +// MockHTTPClient is a mock implementation of HTTPClient +type MockHTTPClient struct { + Response *http.Response + Err error +} + +// Post is the implementation of the Post method for MockHTTPClient +func (m *MockHTTPClient) Post(url string, contentType string, body []byte) (*http.Response, error) { + return m.Response, m.Err +} + +func TestPostDataWithMockClient(t *testing.T) { + // Create a mock HTTP client + mockResponse := &http.Response{ + StatusCode: http.StatusOK, + Body: ioutil.NopCloser(bytes.NewBufferString(`{"status": "success", "OrchestratorType": "Kubernetes", "DncPartitionKey": "1234", "NodeID": "5678"}`)), + Header: make(http.Header), + } + mockClient := &MockHTTPClient{Response: mockResponse, Err: nil} + httpServiceFake := fakes.NewHTTPServiceFake() + + // Make the HTTP request using the mock client + err := sendRegisterNodeRequest(mockClient, httpServiceFake, cns.NodeRegisterRequest{ + NumCores: 2, + NmAgentSupportedApis: nil, + }, "https://localhost:9000/api") + if err != nil { + t.Fatalf("Error making HTTP request: %v", err) + } +} diff --git a/common/utils.go b/common/utils.go index ded8259ada..9e1c3469f9 100644 --- a/common/utils.go +++ b/common/utils.go @@ -81,14 +81,14 @@ type metadataWrapper struct { // HTTPClient interface to abstract http.Client methods type HTTPClient interface { - Post(url string, contentType string, body []byte) (*http.Response, error) + Post(url, contentType string, body []byte) (*http.Response, error) } // StandardHTTPClient is a standard implementation of the HTTPClient interface type StandardHTTPClient struct{} // Post is the implementation of the Post method for StandardHTTPClient -func (s *StandardHTTPClient) Post(url string, contentType string, body []byte) (*http.Response, error) { +func (c *StandardHTTPClient) Post(url string, contentType string, body []byte) (*http.Response, error) { return http.Post(url, contentType, bytes.NewBuffer(body)) } From 9ff3c1bfbbba011176b0ea7e8ea0b1f1fa03dd59 Mon Sep 17 00:00:00 2001 From: Saksham Mittal Date: Fri, 8 Dec 2023 10:17:24 -0800 Subject: [PATCH 04/12] Write test case --- cns/service/main_test.go | 4 ++-- common/utils.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cns/service/main_test.go b/cns/service/main_test.go index e86c09b241..5e75ee133f 100644 --- a/cns/service/main_test.go +++ b/cns/service/main_test.go @@ -4,7 +4,7 @@ import ( "bytes" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/fakes" - "io/ioutil" + "io" "net/http" "testing" ) @@ -24,7 +24,7 @@ func TestPostDataWithMockClient(t *testing.T) { // Create a mock HTTP client mockResponse := &http.Response{ StatusCode: http.StatusOK, - Body: ioutil.NopCloser(bytes.NewBufferString(`{"status": "success", "OrchestratorType": "Kubernetes", "DncPartitionKey": "1234", "NodeID": "5678"}`)), + Body: io.NopCloser(bytes.NewBufferString(`{"status": "success", "OrchestratorType": "Kubernetes", "DncPartitionKey": "1234", "NodeID": "5678"}`)), Header: make(http.Header), } mockClient := &MockHTTPClient{Response: mockResponse, Err: nil} diff --git a/common/utils.go b/common/utils.go index 9e1c3469f9..73296a1848 100644 --- a/common/utils.go +++ b/common/utils.go @@ -88,7 +88,7 @@ type HTTPClient interface { type StandardHTTPClient struct{} // Post is the implementation of the Post method for StandardHTTPClient -func (c *StandardHTTPClient) Post(url string, contentType string, body []byte) (*http.Response, error) { +func (c *StandardHTTPClient) Post(url, contentType string, body []byte) (*http.Response, error) { return http.Post(url, contentType, bytes.NewBuffer(body)) } From 19c82d4381da3076b09016be8c83fc217bf44dd5 Mon Sep 17 00:00:00 2001 From: Saksham Mittal Date: Mon, 11 Dec 2023 11:27:32 -0800 Subject: [PATCH 05/12] Write test cases --- cns/service/main_test.go | 49 ++++++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/cns/service/main_test.go b/cns/service/main_test.go index 5e75ee133f..87db11653f 100644 --- a/cns/service/main_test.go +++ b/cns/service/main_test.go @@ -2,11 +2,16 @@ package main import ( "bytes" - "github.com/Azure/azure-container-networking/cns" - "github.com/Azure/azure-container-networking/cns/fakes" "io" "net/http" "testing" + + "github.com/Azure/azure-container-networking/cns/logger" + + "github.com/stretchr/testify/assert" + + "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/fakes" ) // MockHTTPClient is a mock implementation of HTTPClient @@ -16,26 +21,50 @@ type MockHTTPClient struct { } // Post is the implementation of the Post method for MockHTTPClient -func (m *MockHTTPClient) Post(url string, contentType string, body []byte) (*http.Response, error) { +func (m *MockHTTPClient) Post(url, contentType string, body []byte) (*http.Response, error) { return m.Response, m.Err } -func TestPostDataWithMockClient(t *testing.T) { +func TestSendRegisterNodeRequest_StatusOK(t *testing.T) { + logger.InitLogger("testlogs", 0, 0, "./") + httpServiceFake := fakes.NewHTTPServiceFake() + nodeRegisterReq := cns.NodeRegisterRequest{ + NumCores: 2, + NmAgentSupportedApis: nil, + } + + url := "https://localhost:9000/api" + // Create a mock HTTP client mockResponse := &http.Response{ StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewBufferString(`{"status": "success", "OrchestratorType": "Kubernetes", "DncPartitionKey": "1234", "NodeID": "5678"}`)), Header: make(http.Header), } + mockClient := &MockHTTPClient{Response: mockResponse, Err: nil} - httpServiceFake := fakes.NewHTTPServiceFake() - // Make the HTTP request using the mock client - err := sendRegisterNodeRequest(mockClient, httpServiceFake, cns.NodeRegisterRequest{ + assert.NoError(t, sendRegisterNodeRequest(mockClient, httpServiceFake, nodeRegisterReq, url)) +} + +func TestSendRegisterNodeRequest_StatusAccepted(t *testing.T) { + logger.InitLogger("testlogs", 0, 0, "./") + httpServiceFake := fakes.NewHTTPServiceFake() + nodeRegisterReq := cns.NodeRegisterRequest{ NumCores: 2, NmAgentSupportedApis: nil, - }, "https://localhost:9000/api") - if err != nil { - t.Fatalf("Error making HTTP request: %v", err) } + + url := "https://localhost:9000/api" + + // Create a mock HTTP client + mockResponse := &http.Response{ + StatusCode: http.StatusAccepted, + Body: io.NopCloser(bytes.NewBufferString(`{"status": "accepted", "OrchestratorType": "Kubernetes", "DncPartitionKey": "1234", "NodeID": "5678"}`)), + Header: make(http.Header), + } + + mockClient := &MockHTTPClient{Response: mockResponse, Err: nil} + + assert.Error(t, sendRegisterNodeRequest(mockClient, httpServiceFake, nodeRegisterReq, url)) } From 9a1b590dd94c762943694ff00f05c3321df37f46 Mon Sep 17 00:00:00 2001 From: Saksham Mittal Date: Mon, 11 Dec 2023 11:46:34 -0800 Subject: [PATCH 06/12] Write test cases --- cns/service/main_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/service/main_test.go b/cns/service/main_test.go index 87db11653f..6aeae1d16e 100644 --- a/cns/service/main_test.go +++ b/cns/service/main_test.go @@ -21,7 +21,7 @@ type MockHTTPClient struct { } // Post is the implementation of the Post method for MockHTTPClient -func (m *MockHTTPClient) Post(url, contentType string, body []byte) (*http.Response, error) { +func (m *MockHTTPClient) Post(_, _ string, _ []byte) (*http.Response, error) { return m.Response, m.Err } From afcddaa7fbe646299aa4222fba5720171b231238 Mon Sep 17 00:00:00 2001 From: Saksham Mittal Date: Mon, 11 Dec 2023 12:30:30 -0800 Subject: [PATCH 07/12] Write test cases --- cns/service/main.go | 12 +++++++++--- cns/service/main_test.go | 2 +- common/utils.go | 10 +++++----- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/cns/service/main.go b/cns/service/main.go index b113f1a4b2..90fa65591e 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -416,11 +416,17 @@ func sendRegisterNodeRequest(httpClient acn.HTTPClient, httpRestService cns.HTTP return errors.Wrap(retry.Unrecoverable(err), "failed to sendRegisterNodeRequest") } - response, err := httpClient.Post(registerURL, "application/json", body.Bytes()) + request, err := http.NewRequest(http.MethodPost, registerURL, &body) if err != nil { - logger.Errorf("[Azure CNS] Failed to register node with retryable err: %+v", err) - return errors.Wrap(err, "failed to sendRegisterNodeRequest") + return errors.Wrap(err, "failed to build request") } + + request.Header.Set("Content-Type", "application/json") + response, err := httpClient.Do(request) + if err != nil { + return errors.Wrap(err, "http request failed") + } + defer response.Body.Close() if response.StatusCode != http.StatusOK { diff --git a/cns/service/main_test.go b/cns/service/main_test.go index 6aeae1d16e..9a7e43685d 100644 --- a/cns/service/main_test.go +++ b/cns/service/main_test.go @@ -21,7 +21,7 @@ type MockHTTPClient struct { } // Post is the implementation of the Post method for MockHTTPClient -func (m *MockHTTPClient) Post(_, _ string, _ []byte) (*http.Response, error) { +func (m *MockHTTPClient) Do(_ *http.Request) (*http.Response, error) { return m.Response, m.Err } diff --git a/common/utils.go b/common/utils.go index 73296a1848..8424767294 100644 --- a/common/utils.go +++ b/common/utils.go @@ -4,7 +4,6 @@ package common import ( - "bytes" "context" "encoding/binary" "encoding/json" @@ -81,15 +80,16 @@ type metadataWrapper struct { // HTTPClient interface to abstract http.Client methods type HTTPClient interface { - Post(url, contentType string, body []byte) (*http.Response, error) + Do(req *http.Request) (*http.Response, error) } // StandardHTTPClient is a standard implementation of the HTTPClient interface type StandardHTTPClient struct{} -// Post is the implementation of the Post method for StandardHTTPClient -func (c *StandardHTTPClient) Post(url, contentType string, body []byte) (*http.Response, error) { - return http.Post(url, contentType, bytes.NewBuffer(body)) +// Do is the implementation of the Post method for StandardHTTPClient +func (c *StandardHTTPClient) Do(req *http.Request) (*http.Response, error) { + client := http.Client{} + return client.Do(req) } // Creating http client object to be reused instead of creating one every time. From 4b58f0e40c33bc0e76a94d5eac91ffcaa0cc27b2 Mon Sep 17 00:00:00 2001 From: Saksham Mittal Date: Mon, 11 Dec 2023 12:49:33 -0800 Subject: [PATCH 08/12] Write test cases --- cns/service/main.go | 10 +++++----- cns/service/main_test.go | 13 +++++++------ 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/cns/service/main.go b/cns/service/main.go index 90fa65591e..b25d598452 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -373,7 +373,7 @@ type NodeInterrogator interface { } // RegisterNode - Tries to register node with DNC when CNS is started in managed DNC mode -func registerNode(httpClient acn.HTTPClient, httpRestService cns.HTTPService, dncEP, infraVnet, nodeID string, ni NodeInterrogator) error { +func registerNode(ctx context.Context, httpClient acn.HTTPClient, httpRestService cns.HTTPService, dncEP, infraVnet, nodeID string, ni NodeInterrogator) error { logger.Printf("[Azure CNS] Registering node %s with Infrastructure Network: %s PrivateEndpoint: %s", nodeID, infraVnet, dncEP) var ( @@ -400,7 +400,7 @@ func registerNode(httpClient acn.HTTPClient, httpRestService cns.HTTPService, dn // CNS tries to register Node for maximum of an hour. err := retry.Do(func() error { - return sendRegisterNodeRequest(httpClient, httpRestService, nodeRegisterRequest, url) + return sendRegisterNodeRequest(ctx, httpClient, httpRestService, nodeRegisterRequest, url) }, retry.Delay(acn.FiveSeconds), retry.Attempts(maxRetryNodeRegister), retry.DelayType(retry.FixedDelay)) return errors.Wrap(err, fmt.Sprintf("[Azure CNS] Failed to register node %s after maximum reties for an hour with Infrastructure Network: %s PrivateEndpoint: %s", @@ -408,7 +408,7 @@ func registerNode(httpClient acn.HTTPClient, httpRestService cns.HTTPService, dn } // sendRegisterNodeRequest func helps in registering the node until there is an error. -func sendRegisterNodeRequest(httpClient acn.HTTPClient, httpRestService cns.HTTPService, nodeRegisterRequest cns.NodeRegisterRequest, registerURL string) error { +func sendRegisterNodeRequest(ctx context.Context, httpClient acn.HTTPClient, httpRestService cns.HTTPService, nodeRegisterRequest cns.NodeRegisterRequest, registerURL string) error { var body bytes.Buffer err := json.NewEncoder(&body).Encode(nodeRegisterRequest) if err != nil { @@ -416,7 +416,7 @@ func sendRegisterNodeRequest(httpClient acn.HTTPClient, httpRestService cns.HTTP return errors.Wrap(retry.Unrecoverable(err), "failed to sendRegisterNodeRequest") } - request, err := http.NewRequest(http.MethodPost, registerURL, &body) + request, err := http.NewRequestWithContext(ctx, http.MethodPost, registerURL, &body) if err != nil { return errors.Wrap(err, "failed to build request") } @@ -899,7 +899,7 @@ func main() { standardClient := &acn.StandardHTTPClient{} - registerErr := registerNode(standardClient, httpRestService, privateEndpoint, infravnet, nodeID, nmaClient) + registerErr := registerNode(rootCtx, standardClient, httpRestService, privateEndpoint, infravnet, nodeID, nmaClient) if registerErr != nil { logger.Errorf("[Azure CNS] Resgistering Node failed with error: %v PrivateEndpoint: %s InfrastructureNetworkID: %s NodeID: %s", registerErr, diff --git a/cns/service/main_test.go b/cns/service/main_test.go index 9a7e43685d..42a64df761 100644 --- a/cns/service/main_test.go +++ b/cns/service/main_test.go @@ -2,16 +2,15 @@ package main import ( "bytes" + "context" "io" "net/http" "testing" - "github.com/Azure/azure-container-networking/cns/logger" - - "github.com/stretchr/testify/assert" - "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/fakes" + "github.com/Azure/azure-container-networking/cns/logger" + "github.com/stretchr/testify/assert" ) // MockHTTPClient is a mock implementation of HTTPClient @@ -26,6 +25,7 @@ func (m *MockHTTPClient) Do(_ *http.Request) (*http.Response, error) { } func TestSendRegisterNodeRequest_StatusOK(t *testing.T) { + ctx := context.Background() logger.InitLogger("testlogs", 0, 0, "./") httpServiceFake := fakes.NewHTTPServiceFake() nodeRegisterReq := cns.NodeRegisterRequest{ @@ -44,10 +44,11 @@ func TestSendRegisterNodeRequest_StatusOK(t *testing.T) { mockClient := &MockHTTPClient{Response: mockResponse, Err: nil} - assert.NoError(t, sendRegisterNodeRequest(mockClient, httpServiceFake, nodeRegisterReq, url)) + assert.NoError(t, sendRegisterNodeRequest(ctx, mockClient, httpServiceFake, nodeRegisterReq, url)) } func TestSendRegisterNodeRequest_StatusAccepted(t *testing.T) { + ctx := context.Background() logger.InitLogger("testlogs", 0, 0, "./") httpServiceFake := fakes.NewHTTPServiceFake() nodeRegisterReq := cns.NodeRegisterRequest{ @@ -66,5 +67,5 @@ func TestSendRegisterNodeRequest_StatusAccepted(t *testing.T) { mockClient := &MockHTTPClient{Response: mockResponse, Err: nil} - assert.Error(t, sendRegisterNodeRequest(mockClient, httpServiceFake, nodeRegisterReq, url)) + assert.Error(t, sendRegisterNodeRequest(ctx, mockClient, httpServiceFake, nodeRegisterReq, url)) } From 53756d56926e4c4713eda3441aa482dc6a644c2a Mon Sep 17 00:00:00 2001 From: Saksham Mittal Date: Mon, 11 Dec 2023 12:57:41 -0800 Subject: [PATCH 09/12] Write test cases --- common/utils.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/common/utils.go b/common/utils.go index 8424767294..a756eb3f3b 100644 --- a/common/utils.go +++ b/common/utils.go @@ -18,6 +18,7 @@ import ( "time" "github.com/Azure/azure-container-networking/log" + "github.com/pkg/errors" ) const ( @@ -89,7 +90,12 @@ type StandardHTTPClient struct{} // Do is the implementation of the Post method for StandardHTTPClient func (c *StandardHTTPClient) Do(req *http.Request) (*http.Response, error) { client := http.Client{} - return client.Do(req) + resp, err := client.Do(req) + if err != nil { + return nil, errors.Wrap(err, "http request failed") + } + + return resp, nil } // Creating http client object to be reused instead of creating one every time. From f51a0ee4df5c88e0ac26f34b21e9c25cd6bdb1b4 Mon Sep 17 00:00:00 2001 From: Saksham Mittal Date: Tue, 12 Dec 2023 13:02:33 -0800 Subject: [PATCH 10/12] Write test cases --- cns/service/main.go | 15 ++++++++++----- common/utils.go | 20 -------------------- 2 files changed, 10 insertions(+), 25 deletions(-) diff --git a/cns/service/main.go b/cns/service/main.go index b25d598452..ed9ae403ca 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -372,8 +372,12 @@ type NodeInterrogator interface { SupportedAPIs(context.Context) ([]string, error) } +type httpDoer interface { + Do(req *http.Request) (*http.Response, error) +} + // RegisterNode - Tries to register node with DNC when CNS is started in managed DNC mode -func registerNode(ctx context.Context, httpClient acn.HTTPClient, httpRestService cns.HTTPService, dncEP, infraVnet, nodeID string, ni NodeInterrogator) error { +func registerNode(ctx context.Context, httpClient httpDoer, httpRestService cns.HTTPService, dncEP, infraVnet, nodeID string, ni NodeInterrogator) error { logger.Printf("[Azure CNS] Registering node %s with Infrastructure Network: %s PrivateEndpoint: %s", nodeID, infraVnet, dncEP) var ( @@ -408,7 +412,7 @@ func registerNode(ctx context.Context, httpClient acn.HTTPClient, httpRestServic } // sendRegisterNodeRequest func helps in registering the node until there is an error. -func sendRegisterNodeRequest(ctx context.Context, httpClient acn.HTTPClient, httpRestService cns.HTTPService, nodeRegisterRequest cns.NodeRegisterRequest, registerURL string) error { +func sendRegisterNodeRequest(ctx context.Context, httpClient httpDoer, httpRestService cns.HTTPService, nodeRegisterRequest cns.NodeRegisterRequest, registerURL string) error { var body bytes.Buffer err := json.NewEncoder(&body).Encode(nodeRegisterRequest) if err != nil { @@ -797,7 +801,7 @@ func main() { } // We might be configured to reinitialize state from the CNI instead of the apiserver. - // If so, we should check that the the CNI is new enough to support the state commands, + // If so, we should check that the CNI is new enough to support the state commands, // otherwise we fall back to the existing behavior. if cnsconfig.InitializeFromCNI { var isGoodVer bool @@ -897,11 +901,12 @@ func main() { httpRestService.SetOption(acn.OptInfrastructureNetworkID, infravnet) httpRestService.SetOption(acn.OptNodeID, nodeID) - standardClient := &acn.StandardHTTPClient{} + // Passing in the default http client that already implements Do function + standardClient := http.DefaultClient registerErr := registerNode(rootCtx, standardClient, httpRestService, privateEndpoint, infravnet, nodeID, nmaClient) if registerErr != nil { - logger.Errorf("[Azure CNS] Resgistering Node failed with error: %v PrivateEndpoint: %s InfrastructureNetworkID: %s NodeID: %s", + logger.Errorf("[Azure CNS] Registering Node failed with error: %v PrivateEndpoint: %s InfrastructureNetworkID: %s NodeID: %s", registerErr, privateEndpoint, infravnet, diff --git a/common/utils.go b/common/utils.go index a756eb3f3b..cd6a5bffa0 100644 --- a/common/utils.go +++ b/common/utils.go @@ -18,7 +18,6 @@ import ( "time" "github.com/Azure/azure-container-networking/log" - "github.com/pkg/errors" ) const ( @@ -79,25 +78,6 @@ type metadataWrapper struct { Metadata Metadata `json:"compute"` } -// HTTPClient interface to abstract http.Client methods -type HTTPClient interface { - Do(req *http.Request) (*http.Response, error) -} - -// StandardHTTPClient is a standard implementation of the HTTPClient interface -type StandardHTTPClient struct{} - -// Do is the implementation of the Post method for StandardHTTPClient -func (c *StandardHTTPClient) Do(req *http.Request) (*http.Response, error) { - client := http.Client{} - resp, err := client.Do(req) - if err != nil { - return nil, errors.Wrap(err, "http request failed") - } - - return resp, nil -} - // Creating http client object to be reused instead of creating one every time. // This helps make use of the cached tcp connections. // Clients are safe for concurrent use by multiple goroutines. From d218420268975a1d36c4a5f7c7a610b50b64222d Mon Sep 17 00:00:00 2001 From: Saksham Mittal Date: Tue, 12 Dec 2023 13:10:25 -0800 Subject: [PATCH 11/12] Write test cases --- cns/service/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/service/main.go b/cns/service/main.go index ed9ae403ca..b9c3a3ccf6 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -404,7 +404,7 @@ func registerNode(ctx context.Context, httpClient httpDoer, httpRestService cns. // CNS tries to register Node for maximum of an hour. err := retry.Do(func() error { - return sendRegisterNodeRequest(ctx, httpClient, httpRestService, nodeRegisterRequest, url) + return errors.Wrap(sendRegisterNodeRequest(ctx, httpClient, httpRestService, nodeRegisterRequest, url), "failed to build request") }, retry.Delay(acn.FiveSeconds), retry.Attempts(maxRetryNodeRegister), retry.DelayType(retry.FixedDelay)) return errors.Wrap(err, fmt.Sprintf("[Azure CNS] Failed to register node %s after maximum reties for an hour with Infrastructure Network: %s PrivateEndpoint: %s", From 0fd27902d3606cf1ed7e63680c933bedca7342de Mon Sep 17 00:00:00 2001 From: Saksham Mittal Date: Tue, 12 Dec 2023 13:22:55 -0800 Subject: [PATCH 12/12] fix linter issues --- cns/service/main.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cns/service/main.go b/cns/service/main.go index b9c3a3ccf6..81890232aa 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -390,12 +390,11 @@ func registerNode(ctx context.Context, httpClient httpDoer, httpRestService cns. supportedApis, retErr := ni.SupportedAPIs(context.TODO()) if retErr != nil { - logger.Errorf("[Azure CNS] Failed to retrieve SupportedApis from NMagent of node %s with Infrastructure Network: %s PrivateEndpoint: %s", - nodeID, infraVnet, dncEP) - return retErr + return errors.Wrap(retErr, fmt.Sprintf("[Azure CNS] Failed to retrieve SupportedApis from NMagent of node %s with Infrastructure Network: %s PrivateEndpoint: %s", + nodeID, infraVnet, dncEP)) } - // To avoid any null-pointer deferencing errors. + // To avoid any null-pointer de-referencing errors. if supportedApis == nil { supportedApis = []string{} } @@ -404,7 +403,7 @@ func registerNode(ctx context.Context, httpClient httpDoer, httpRestService cns. // CNS tries to register Node for maximum of an hour. err := retry.Do(func() error { - return errors.Wrap(sendRegisterNodeRequest(ctx, httpClient, httpRestService, nodeRegisterRequest, url), "failed to build request") + return errors.Wrap(sendRegisterNodeRequest(ctx, httpClient, httpRestService, nodeRegisterRequest, url), "failed to sendRegisterNodeRequest") }, retry.Delay(acn.FiveSeconds), retry.Attempts(maxRetryNodeRegister), retry.DelayType(retry.FixedDelay)) return errors.Wrap(err, fmt.Sprintf("[Azure CNS] Failed to register node %s after maximum reties for an hour with Infrastructure Network: %s PrivateEndpoint: %s",