From 1ddc707299d59644fa6ca5692f40be6b41a6548a Mon Sep 17 00:00:00 2001 From: bohuini Date: Wed, 14 Sep 2022 11:45:45 -0700 Subject: [PATCH 1/5] NC refresh optimization: Added api and test --- cns/NetworkContainerContract.go | 29 ++++-- cns/restserver/api.go | 77 +++++++------- cns/restserver/api_test.go | 172 ++++++++++++++++++++++++++++++++ cns/restserver/restserver.go | 1 + cns/restserver/util.go | 87 ++++++++++++++++ 5 files changed, 320 insertions(+), 46 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 8b47eb14df..b544bd8dc7 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -17,11 +17,11 @@ const ( SetOrchestratorType = "/network/setorchestratortype" CreateOrUpdateNetworkContainer = "/network/createorupdatenetworkcontainer" DeleteNetworkContainer = "/network/deletenetworkcontainer" - GetNetworkContainerStatus = "/network/getnetworkcontainerstatus" PublishNetworkContainer = "/network/publishnetworkcontainer" UnpublishNetworkContainer = "/network/unpublishnetworkcontainer" GetInterfaceForContainer = "/network/getinterfaceforcontainer" GetNetworkContainerByOrchestratorContext = "/network/getnetworkcontainerbyorchestratorcontext" + NetworkContainersURLPath = "/network/networkcontainers" AttachContainerToNetwork = "/network/attachcontainertonetwork" DetachContainerFromNetwork = "/network/detachcontainerfromnetwork" RequestIPConfig = "/network/requestipconfig" @@ -340,12 +340,12 @@ type CreateNetworkContainerResponse struct { Response Response } -// GetNetworkContainerStatusRequest specifies the details about the request to retrieve status of a specifc network container. +// GetNetworkContainerStatusRequest specifies the details about the request to retrieve status of a specific network container. type GetNetworkContainerStatusRequest struct { NetworkContainerid string } -// GetNetworkContainerStatusResponse specifies response of retriving a network container status. +// GetNetworkContainerStatusResponse specifies response of retrieving a network container status. type GetNetworkContainerStatusResponse struct { NetworkContainerid string Version string @@ -353,13 +353,26 @@ type GetNetworkContainerStatusResponse struct { Response Response } -// GetNetworkContainerRequest specifies the details about the request to retrieve a specifc network container. +type GetAllNetworkContainersResponse struct { + NetworkContainers []GetNetworkContainerResponse + Response Response +} + +type PostNetworkContainersRequest struct { + CreateNetworkContainerRequests []CreateNetworkContainerRequest +} + +type PostNetworkContainersResponse struct { + Response Response +} + +// GetNetworkContainerRequest specifies the details about the request to retrieve a specific network container. type GetNetworkContainerRequest struct { NetworkContainerid string OrchestratorContext json.RawMessage } -// GetNetworkContainerResponse describes the response to retrieve a specifc network container. +// GetNetworkContainerResponse describes the response to retrieve a specific network container. type GetNetworkContainerResponse struct { NetworkContainerID string IPConfiguration IPConfiguration @@ -436,12 +449,12 @@ type IPAddressState struct { State string } -// DeleteNetworkContainerRequest specifies the details about the request to delete a specifc network container. +// DeleteNetworkContainerRequest specifies the details about the request to delete a specific network container. type DeleteNetworkContainerRequest struct { NetworkContainerid string } -// DeleteNetworkContainerResponse describes the response to delete a specifc network container. +// DeleteNetworkContainerResponse describes the response to delete a specific network container. type DeleteNetworkContainerResponse struct { Response Response } @@ -470,7 +483,7 @@ type DetachContainerFromNetworkResponse struct { Response Response } -// NetworkInterface specifies the information that can be used to unquely identify an interface. +// NetworkInterface specifies the information that can be used to uniquely identify an interface. type NetworkInterface struct { Name string IPAddress string diff --git a/cns/restserver/api.go b/cns/restserver/api.go index c2461d599f..6753285e5b 100644 --- a/cns/restserver/api.go +++ b/cns/restserver/api.go @@ -5,7 +5,6 @@ package restserver import ( "context" - "errors" "fmt" "io" "net" @@ -23,6 +22,7 @@ import ( "github.com/Azure/azure-container-networking/cns/wireserver" "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/platform" + "github.com/pkg/errors" ) var ( @@ -54,7 +54,7 @@ func (service *HTTPRestService) setEnvironment(w http.ResponseWriter, r *http.Re } switch r.Method { - case "POST": + case http.MethodPost: logger.Printf("[Azure CNS] POST received for SetEnvironment.") service.state.Location = req.Location service.state.NetworkType = req.NetworkType @@ -88,7 +88,7 @@ func (service *HTTPRestService) createNetwork(w http.ResponseWriter, r *http.Req returnCode = types.InvalidParameter } else { switch r.Method { - case "POST": + case http.MethodPost: dc := service.dockerClient rt := service.routingTable err = dc.NetworkExists(req.NetworkName) @@ -190,7 +190,7 @@ func (service *HTTPRestService) deleteNetwork(w http.ResponseWriter, r *http.Req } switch r.Method { - case "POST": + case http.MethodPost: dc := service.dockerClient err := dc.NetworkExists(req.NetworkName) @@ -249,7 +249,7 @@ func (service *HTTPRestService) createHnsNetwork(w http.ResponseWriter, r *http. returnCode = types.InvalidParameter } else { switch r.Method { - case "POST": + case http.MethodPost: if err := hnsclient.CreateHnsNetwork(req); err == nil { // Save the newly created HnsNetwork name. CNS deleteHnsNetwork API // will only allow deleting these networks. @@ -300,7 +300,7 @@ func (service *HTTPRestService) deleteHnsNetwork(w http.ResponseWriter, r *http. returnCode = types.InvalidParameter } else { switch r.Method { - case "POST": + case http.MethodPost: networkInfo, found := service.getNetworkInfo(req.NetworkName) if found && networkInfo.NetworkName == req.NetworkName { if err = hnsclient.DeleteHnsNetwork(req.NetworkName); err == nil { @@ -357,7 +357,7 @@ func (service *HTTPRestService) reserveIPAddress(w http.ResponseWriter, r *http. } switch r.Method { - case "POST": + case http.MethodPost: ic := service.ipamClient var ifInfo *wireserver.InterfaceInfo @@ -434,7 +434,7 @@ func (service *HTTPRestService) releaseIPAddress(w http.ResponseWriter, r *http. } switch r.Method { - case "POST": + case http.MethodPost: ic := service.ipamClient var ifInfo *wireserver.InterfaceInfo @@ -490,7 +490,7 @@ func (service *HTTPRestService) getHostLocalIP(w http.ResponseWriter, r *http.Re if service.state.Initialized { switch r.Method { - case "GET": + case http.MethodGet: switch service.state.NetworkType { case "Underlay": if service.wscli != nil { @@ -543,7 +543,7 @@ func (service *HTTPRestService) getIPAddressUtilization(w http.ResponseWriter, r var unhealthyAddrs []string switch r.Method { - case "GET": + case http.MethodGet: ic := service.ipamClient ifInfo, err := service.getPrimaryHostInterface(context.TODO()) @@ -601,11 +601,6 @@ func (service *HTTPRestService) getAvailableIPAddresses(w http.ResponseWriter, r logger.Printf("[Azure CNS] getAvailableIPAddresses") logger.Request(service.Name, "getAvailableIPAddresses", nil) - switch r.Method { - case "GET": - default: - } - resp := cns.Response{ReturnCode: 0} ipResp := &cns.GetIPAddressesResponse{Response: resp} err := service.Listener.Encode(w, &ipResp) @@ -618,11 +613,6 @@ func (service *HTTPRestService) getReservedIPAddresses(w http.ResponseWriter, r logger.Printf("[Azure CNS] getReservedIPAddresses") logger.Request(service.Name, "getReservedIPAddresses", nil) - switch r.Method { - case "GET": - default: - } - resp := cns.Response{ReturnCode: 0} ipResp := &cns.GetIPAddressesResponse{Response: resp} err := service.Listener.Encode(w, &ipResp) @@ -642,7 +632,7 @@ func (service *HTTPRestService) getUnhealthyIPAddresses(w http.ResponseWriter, r var unhealthyAddrs []string switch r.Method { - case "GET": + case http.MethodGet: ic := service.ipamClient ifInfo, err := service.getPrimaryHostInterface(context.TODO()) @@ -698,11 +688,6 @@ func (service *HTTPRestService) getAllIPAddresses(w http.ResponseWriter, r *http logger.Printf("[Azure CNS] getAllIPAddresses") logger.Request(service.Name, "getAllIPAddresses", nil) - switch r.Method { - case "GET": - default: - } - resp := cns.Response{ReturnCode: 0} ipResp := &cns.GetIPAddressesResponse{Response: resp} err := service.Listener.Encode(w, &ipResp) @@ -715,11 +700,6 @@ func (service *HTTPRestService) getHealthReport(w http.ResponseWriter, r *http.R logger.Printf("[Azure CNS] getHealthReport") logger.Request(service.Name, "getHealthReport", nil) - switch r.Method { - case "GET": - default: - } - resp := &cns.Response{ReturnCode: 0} err := service.Listener.Encode(w, &resp) @@ -798,7 +778,7 @@ func (service *HTTPRestService) createOrUpdateNetworkContainer(w http.ResponseWr var returnCode types.ResponseCode var returnMessage string switch r.Method { - case "POST": + case http.MethodPost: if req.NetworkContainerType == cns.WebApps { // try to get the saved nc state if it exists existing, ok := service.getNetworkContainerDetails(req.NetworkContainerid) @@ -899,6 +879,27 @@ func (service *HTTPRestService) getNetworkContainerByOrchestratorContext(w http. logger.Response(service.Name, getNetworkContainerResponse, returnCode, err) } +// getOrRefreshNetworkContainers is to check whether refresh association is needed. +// If received "GET": Return all NCs in CNS's state file to DNC in order to check if NC refresh is needed +// If received "POST": Store all the NCs (from the request body that client sent) into CNS's state file +func (service *HTTPRestService) getOrRefreshNetworkContainers(w http.ResponseWriter, r *http.Request) { + logger.Printf("[Azure CNS] getOrRefreshNetworkContainers") + + switch r.Method { + case http.MethodGet: + service.handleGetNetworkContainers(w) + return + case http.MethodPost: + service.handlePostNetworkContainers(w, r) + return + default: + w.WriteHeader(http.StatusMethodNotAllowed) + err := errors.New("[Azure CNS] getOrRefreshNetworkContainers did not receive a GET or POST") + logger.Response(service.Name, nil, types.InvalidParameter, err) + return + } +} + func (service *HTTPRestService) deleteNetworkContainer(w http.ResponseWriter, r *http.Request) { logger.Printf("[Azure CNS] deleteNetworkContainer") @@ -918,7 +919,7 @@ func (service *HTTPRestService) deleteNetworkContainer(w http.ResponseWriter, r } switch r.Method { - case "POST": + case http.MethodPost: var containerStatus containerstatus var ok bool @@ -1069,7 +1070,7 @@ func (service *HTTPRestService) getNumberOfCPUCores(w http.ResponseWriter, r *ht ) switch r.Method { - case "GET": + case http.MethodGet: num = runtime.NumCPU() default: errMsg = "[Azure-CNS] getNumberOfCPUCores API expects a GET." @@ -1170,7 +1171,7 @@ func (service *HTTPRestService) publishNetworkContainer(w http.ResponseWriter, r } switch r.Method { - case "POST": + case http.MethodPost: // Join the network // Please refactor this // do not reuse the below variable between network join and publish @@ -1298,7 +1299,7 @@ func (service *HTTPRestService) unpublishNetworkContainer(w http.ResponseWriter, } switch r.Method { - case "POST": + case http.MethodPost: // Join Network if not joined already isNetworkJoined = service.isNetworkJoined(req.NetworkID) if !isNetworkJoined { @@ -1384,7 +1385,7 @@ func (service *HTTPRestService) createHostNCApipaEndpoint(w http.ResponseWriter, } switch r.Method { - case "POST": + case http.MethodPost: networkContainerDetails, found := service.getNetworkContainerDetails(req.NetworkContainerID) if found { if !networkContainerDetails.CreateNetworkContainerRequest.AllowNCToHostCommunication && @@ -1442,7 +1443,7 @@ func (service *HTTPRestService) deleteHostNCApipaEndpoint(w http.ResponseWriter, } switch r.Method { - case "POST": + case http.MethodPost: if err = hnsclient.DeleteHostNCApipaEndpoint(req.NetworkContainerID); err != nil { returnMessage = fmt.Sprintf("Failed to delete endpoint for Network Container: %s "+ "due to error: %v", req.NetworkContainerID, err) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index a9ecae7e2c..5512ce3e28 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -8,6 +8,7 @@ import ( "context" "encoding/json" "encoding/xml" + "errors" "fmt" "net/http" "net/http/httptest" @@ -82,6 +83,25 @@ var ( }, }}, } + + nc1 = createOrUpdateNetworkContainerParams{ + ncID: "ethWebApp1", + ncIP: "11.0.0.5", + ncType: cns.AzureContainerInstance, + ncVersion: "0", + podName: "testpod", + podNamespace: "testpodnamespace", + } + nc2 = createOrUpdateNetworkContainerParams{ + ncID: "ethWebApp2", + ncIP: "11.0.0.5", + ncType: cns.AzureContainerInstance, + ncVersion: "0", + podName: "testpod", + podNamespace: "testpodnamespace", + } + ncParams = []createOrUpdateNetworkContainerParams{nc1, nc2} + errMismatchedNCs = errors.New("GetNetworkContainers failed because NCs not matched") ) const ( @@ -828,6 +848,149 @@ func TestCreateHostNCApipaEndpoint(t *testing.T) { fmt.Printf("createHostNCApipaEndpoint Responded with %+v\n", createHostNCApipaEndpointResponse) } +func TestGetNetworkContainers(t *testing.T) { + t.Run("Test Get Network Containers", func(t *testing.T) { + setEnv(t) + err := setOrchestratorType(t, cns.Kubernetes) + if err != nil { + t.Errorf("TestGetNetworkContainers failed with error:%+v", err) + t.Fatal(err) + } + + for i := 0; i < len(ncParams); i++ { + err = createOrUpdateNetworkContainerWithParams(t, ncParams[i]) + if err != nil { + t.Errorf("createOrUpdateNetworkContainerWithParams failed with error:%+v", err) + t.Fatal(err) + } + } + + err = getAllNetworkContainers(ncParams) + if err != nil { + t.Errorf("TestGetNetworkContainers failed with error:%+v", err) + t.Fatal(err) + } + + for i := 0; i < len(ncParams); i++ { + err = deleteNetworkContainerWithParams(t, ncParams[i]) + if err != nil { + t.Errorf("createOrUpdateNetworkContainerWithParams failed with error:%+v", err) + t.Fatal(err) + } + } + }) +} + +func getAllNetworkContainers(ncParams []createOrUpdateNetworkContainerParams) error { + req, err := http.NewRequestWithContext(context.TODO(), http.MethodGet, cns.NetworkContainersURLPath, http.NoBody) + if err != nil { + return fmt.Errorf("GetNetworkContainers failed with error: %w", err) + } + + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + var resp cns.GetAllNetworkContainersResponse + err = decodeResponse(w, &resp) + if err != nil || resp.Response.ReturnCode != types.Success || len(resp.NetworkContainers) != len(ncParams) { + return fmt.Errorf("GetNetworkContainers failed with response %+v Err: %w", resp, err) + } + + // If any NC in response is not found in ncParams, it means get all NCs failed + for i := 0; i < len(ncParams); i++ { + if !contains(resp.NetworkContainers, cns.SwiftPrefix+ncParams[i].ncID) { + return errMismatchedNCs + } + } + + fmt.Printf("GetNetworkContainers succeeded with response %+v, raw:%+v\n", resp, w.Body) + return nil +} + +func TestPostNetworkContainers(t *testing.T) { + t.Run("Test Post Network Containers", func(t *testing.T) { + setEnv(t) + err := setOrchestratorType(t, cns.Kubernetes) + if err != nil { + t.Errorf("TestPostNetworkContainers failed with error:%+v", err) + t.Fatal(err) + } + + err = postAllNetworkContainers(ncParams) + if err != nil { + t.Errorf("Failed to save all network containers due to error: %+v", err) + t.Fatal(err) + } + + err = getAllNetworkContainers(ncParams) + if err != nil { + t.Errorf("TestPostNetworkContainers failed with error:%+v", err) + t.Fatal(err) + } + + for i := 0; i < len(ncParams); i++ { + err = deleteNetworkContainerWithParams(t, ncParams[i]) + if err != nil { + t.Errorf("TestPostNetworkContainers failed with error:%+v", err) + t.Fatal(err) + } + } + }) +} + +func postAllNetworkContainers(ncParams []createOrUpdateNetworkContainerParams) error { + var ipConfig cns.IPConfiguration + ipConfig.DNSServers = []string{"8.8.8.8", "8.8.4.4"} + ipConfig.GatewayIPAddress = "11.0.0.1" + podInfo := cns.KubernetesPodInfo{PodName: "testpod", PodNamespace: "testpodnamespace"} + ctx, err := json.Marshal(podInfo) + if err != nil { + return fmt.Errorf("postAllNetworkContainers failed with error: %w", err) + } + createReq := make([]cns.CreateNetworkContainerRequest, len(ncParams)) + postReq := cns.PostNetworkContainersRequest{CreateNetworkContainerRequests: createReq} + + for i := 0; i < len(ncParams); i++ { + var ipSubnet cns.IPSubnet + ipSubnet.IPAddress = ncParams[i].ncIP + ipSubnet.PrefixLength = 24 + ipConfig.IPSubnet = ipSubnet + + postReq.CreateNetworkContainerRequests[i] = cns.CreateNetworkContainerRequest{ + Version: ncParams[i].ncVersion, + NetworkContainerType: ncParams[i].ncType, + NetworkContainerid: cns.SwiftPrefix + ncParams[i].ncID, + OrchestratorContext: ctx, + IPConfiguration: ipConfig, + PrimaryInterfaceIdentifier: "11.0.0.7", + } + } + + var body bytes.Buffer + err = json.NewEncoder(&body).Encode(postReq) + if err != nil { + return fmt.Errorf("postAllNetworkContainers failed with error: %w", err) + } + + req, err := http.NewRequestWithContext(context.TODO(), http.MethodPost, cns.NetworkContainersURLPath, &body) + if err != nil { + return fmt.Errorf("postAllNetworkContainers failed with error: %w", err) + } + + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + var resp cns.PostNetworkContainersResponse + err = decodeResponse(w, &resp) + fmt.Printf("Raw response: %+v", w.Body) + + if err != nil || resp.Response.ReturnCode != types.Success { + return fmt.Errorf("post Network Containers failed with response %+v Err: %w", resp, err) + } + fmt.Printf("Post Network Containers succeeded with response %+v\n", resp) + + return nil +} + func setOrchestratorType(t *testing.T, orchestratorType string) error { var body bytes.Buffer @@ -1148,6 +1311,15 @@ func startService() error { return nil } +func contains(networkContainers []cns.GetNetworkContainerResponse, str string) bool { + for i := 0; i < len(networkContainers); i++ { + if networkContainers[i].NetworkContainerID == str { + return true + } + } + return false +} + // IGNORE TEST AS IT IS FAILING. TODO:- Fix it https://msazure.visualstudio.com/One/_workitems/edit/7720083 // // Tests CreateNetwork functionality. diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index 5106583e66..f916340e21 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -223,6 +223,7 @@ func (service *HTTPRestService) Init(config *common.ServiceConfig) error { listener.AddHandler(cns.PathDebugIPAddresses, service.handleDebugIPAddresses) listener.AddHandler(cns.PathDebugPodContext, service.handleDebugPodContext) listener.AddHandler(cns.PathDebugRestData, service.handleDebugRestData) + listener.AddHandler(cns.NetworkContainersURLPath, service.getOrRefreshNetworkContainers) // handlers for v0.2 listener.AddHandler(cns.V2Prefix+cns.SetEnvironmentPath, service.setEnvironment) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 05acd5701e..c30235cd84 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -840,3 +840,90 @@ func (service *HTTPRestService) isNCWaitingForUpdate( } return } + +// handleGetNetworkContainers returns all NCs in CNS +func (service *HTTPRestService) handleGetNetworkContainers(w http.ResponseWriter) { + service.RLock() + networkContainers := make([]cns.GetNetworkContainerResponse, len(service.state.ContainerStatus)) + i := 0 + for ncID := range service.state.ContainerStatus { + ncDetails := service.state.ContainerStatus[ncID] + getNcResp := cns.GetNetworkContainerResponse{ + NetworkContainerID: ncDetails.CreateNetworkContainerRequest.NetworkContainerid, + IPConfiguration: ncDetails.CreateNetworkContainerRequest.IPConfiguration, + Routes: ncDetails.CreateNetworkContainerRequest.Routes, + CnetAddressSpace: ncDetails.CreateNetworkContainerRequest.CnetAddressSpace, + MultiTenancyInfo: ncDetails.CreateNetworkContainerRequest.MultiTenancyInfo, + PrimaryInterfaceIdentifier: ncDetails.CreateNetworkContainerRequest.PrimaryInterfaceIdentifier, + LocalIPConfiguration: ncDetails.CreateNetworkContainerRequest.LocalIPConfiguration, + AllowHostToNCCommunication: ncDetails.CreateNetworkContainerRequest.AllowHostToNCCommunication, + AllowNCToHostCommunication: ncDetails.CreateNetworkContainerRequest.AllowNCToHostCommunication, + } + networkContainers[i] = getNcResp + i++ + } + service.RUnlock() + + response := cns.GetAllNetworkContainersResponse{ + NetworkContainers: networkContainers, + Response: cns.Response{ + ReturnCode: types.Success, + }, + } + err := service.Listener.Encode(w, &response) + logger.Response(service.Name, response, response.Response.ReturnCode, err) +} + +// handlePostNetworkContainers stores all the NCs (from the request that client sent) into CNS's state file +func (service *HTTPRestService) handlePostNetworkContainers(w http.ResponseWriter, r *http.Request) { + var req cns.PostNetworkContainersRequest + err := service.Listener.Decode(w, r, &req) + logger.Request(service.Name, &req, err) + if err != nil { + response := cns.PostNetworkContainersResponse{ + Response: cns.Response{ + ReturnCode: types.InvalidRequest, + Message: fmt.Sprintf("[Azure CNS] Create Network Container failed with error: %s", err.Error()), + }, + } + err = service.Listener.Encode(w, &response) + logger.Response(service.Name, response, response.Response.ReturnCode, err) + return + } + + returnCode := types.Success + returnMessage := "" + for i := 0; i < len(req.CreateNetworkContainerRequests); i++ { + createNcReq := req.CreateNetworkContainerRequests[i] + ncDetails, found := service.getNetworkContainerDetails(createNcReq.NetworkContainerid) + // Create NC if it doesn't exist, or it exists and the requested version is different from the saved version + if !found || (found && ncDetails.VMVersion != createNcReq.Version) { + nc := service.networkContainer + if err = nc.Create(createNcReq); err != nil { + returnCode = types.UnexpectedError + returnMessage = fmt.Sprintf("[Azure CNS] Create Network Container failed with error: %s", err.Error()) + break + } + } + // Save NC Goal State details + saveNcReturnCode, saveNcReturnMessage := service.saveNetworkContainerGoalState(createNcReq) + + // If NC was created successfully, log NC snapshot. + if saveNcReturnCode == types.Success { + logNCSnapshot(createNcReq) + } else { + returnCode = saveNcReturnCode + returnMessage = saveNcReturnMessage + break + } + } + + response := cns.PostNetworkContainersResponse{ + Response: cns.Response{ + ReturnCode: returnCode, + Message: returnMessage, + }, + } + err = service.Listener.Encode(w, &response) + logger.Response(service.Name, response, response.Response.ReturnCode, err) +} From 9548c2c02d3891c55a7c5ea9ce0be61b8b9314f1 Mon Sep 17 00:00:00 2001 From: bohuini Date: Thu, 15 Sep 2022 12:23:42 -0700 Subject: [PATCH 2/5] NC refresh optimization: Address comments --- cns/NetworkContainerContract.go | 7 +-- cns/restserver/api_test.go | 93 ++++++++++++++------------------- cns/restserver/util.go | 38 ++++++++------ 3 files changed, 66 insertions(+), 72 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index b544bd8dc7..77ba451414 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -305,7 +305,7 @@ type IPConfiguration struct { // SecondaryIPConfig contains IP info of SecondaryIP type SecondaryIPConfig struct { IPAddress string - // NCVesion will help in determining whether IP is in pending programming or available when reconciling. + // NCVersion will help in determining whether IP is in pending programming or available when reconciling. NCVersion int } @@ -353,15 +353,18 @@ type GetNetworkContainerStatusResponse struct { Response Response } +// GetAllNetworkContainersResponse specifies response of retrieving all NCs from CNS during the process of NC refresh association. type GetAllNetworkContainersResponse struct { NetworkContainers []GetNetworkContainerResponse Response Response } +// PostNetworkContainersRequest specifies the request of creating all NCs that are sent from DNC. type PostNetworkContainersRequest struct { CreateNetworkContainerRequests []CreateNetworkContainerRequest } +// PostNetworkContainersResponse specifies response of creating all NCs that are sent from DNC. type PostNetworkContainersResponse struct { Response Response } @@ -386,14 +389,12 @@ type GetNetworkContainerResponse struct { AllowNCToHostCommunication bool } -// DeleteNetworkContainerRequest specifies the details about the request to delete a specifc network container. type PodIpInfo struct { PodIPConfig IPSubnet NetworkContainerPrimaryIPConfig IPConfiguration HostPrimaryIPInfo HostIPInfo } -// DeleteNetworkContainerRequest specifies the details about the request to delete a specifc network container. type HostIPInfo struct { Gateway string PrimaryIP string diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 5512ce3e28..582f50d23d 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -849,39 +849,33 @@ func TestCreateHostNCApipaEndpoint(t *testing.T) { } func TestGetNetworkContainers(t *testing.T) { - t.Run("Test Get Network Containers", func(t *testing.T) { - setEnv(t) - err := setOrchestratorType(t, cns.Kubernetes) + setEnv(t) + err := setOrchestratorType(t, cns.Kubernetes) + if err != nil { + t.Fatalf("TestGetNetworkContainers failed with error:%+v", err) + } + + for i := 0; i < len(ncParams); i++ { + err = createOrUpdateNetworkContainerWithParams(t, ncParams[i]) if err != nil { - t.Errorf("TestGetNetworkContainers failed with error:%+v", err) - t.Fatal(err) + t.Fatalf("createOrUpdateNetworkContainerWithParams failed with error:%+v", err) } + } - for i := 0; i < len(ncParams); i++ { - err = createOrUpdateNetworkContainerWithParams(t, ncParams[i]) - if err != nil { - t.Errorf("createOrUpdateNetworkContainerWithParams failed with error:%+v", err) - t.Fatal(err) - } - } + err = getAllNetworkContainers(t, ncParams) + if err != nil { + t.Fatalf("TestGetNetworkContainers failed with error:%+v", err) + } - err = getAllNetworkContainers(ncParams) + for i := 0; i < len(ncParams); i++ { + err = deleteNetworkContainerWithParams(t, ncParams[i]) if err != nil { - t.Errorf("TestGetNetworkContainers failed with error:%+v", err) - t.Fatal(err) + t.Fatalf("createOrUpdateNetworkContainerWithParams failed with error:%+v", err) } - - for i := 0; i < len(ncParams); i++ { - err = deleteNetworkContainerWithParams(t, ncParams[i]) - if err != nil { - t.Errorf("createOrUpdateNetworkContainerWithParams failed with error:%+v", err) - t.Fatal(err) - } - } - }) + } } -func getAllNetworkContainers(ncParams []createOrUpdateNetworkContainerParams) error { +func getAllNetworkContainers(t *testing.T, ncParams []createOrUpdateNetworkContainerParams) error { req, err := http.NewRequestWithContext(context.TODO(), http.MethodGet, cns.NetworkContainersURLPath, http.NoBody) if err != nil { return fmt.Errorf("GetNetworkContainers failed with error: %w", err) @@ -903,42 +897,36 @@ func getAllNetworkContainers(ncParams []createOrUpdateNetworkContainerParams) er } } - fmt.Printf("GetNetworkContainers succeeded with response %+v, raw:%+v\n", resp, w.Body) + t.Logf("GetNetworkContainers succeeded with response: %+v", resp) return nil } func TestPostNetworkContainers(t *testing.T) { - t.Run("Test Post Network Containers", func(t *testing.T) { - setEnv(t) - err := setOrchestratorType(t, cns.Kubernetes) - if err != nil { - t.Errorf("TestPostNetworkContainers failed with error:%+v", err) - t.Fatal(err) - } + setEnv(t) + err := setOrchestratorType(t, cns.Kubernetes) + if err != nil { + t.Fatalf("TestPostNetworkContainers failed with error:%+v", err) + } - err = postAllNetworkContainers(ncParams) - if err != nil { - t.Errorf("Failed to save all network containers due to error: %+v", err) - t.Fatal(err) - } + err = postAllNetworkContainers(t, ncParams) + if err != nil { + t.Fatalf("Failed to save all network containers due to error: %+v", err) + } - err = getAllNetworkContainers(ncParams) - if err != nil { - t.Errorf("TestPostNetworkContainers failed with error:%+v", err) - t.Fatal(err) - } + err = getAllNetworkContainers(t, ncParams) + if err != nil { + t.Fatalf("TestPostNetworkContainers failed with error:%+v", err) + } - for i := 0; i < len(ncParams); i++ { - err = deleteNetworkContainerWithParams(t, ncParams[i]) - if err != nil { - t.Errorf("TestPostNetworkContainers failed with error:%+v", err) - t.Fatal(err) - } + for i := 0; i < len(ncParams); i++ { + err = deleteNetworkContainerWithParams(t, ncParams[i]) + if err != nil { + t.Fatalf("TestPostNetworkContainers failed with error:%+v", err) } - }) + } } -func postAllNetworkContainers(ncParams []createOrUpdateNetworkContainerParams) error { +func postAllNetworkContainers(t *testing.T, ncParams []createOrUpdateNetworkContainerParams) error { var ipConfig cns.IPConfiguration ipConfig.DNSServers = []string{"8.8.8.8", "8.8.4.4"} ipConfig.GatewayIPAddress = "11.0.0.1" @@ -981,12 +969,11 @@ func postAllNetworkContainers(ncParams []createOrUpdateNetworkContainerParams) e mux.ServeHTTP(w, req) var resp cns.PostNetworkContainersResponse err = decodeResponse(w, &resp) - fmt.Printf("Raw response: %+v", w.Body) if err != nil || resp.Response.ReturnCode != types.Success { return fmt.Errorf("post Network Containers failed with response %+v Err: %w", resp, err) } - fmt.Printf("Post Network Containers succeeded with response %+v\n", resp) + t.Logf("Post Network Containers succeeded with response %+v\n", resp) return nil } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index c30235cd84..b91cdc623d 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -891,15 +891,25 @@ func (service *HTTPRestService) handlePostNetworkContainers(w http.ResponseWrite return } + createNCsResp := service.createNetworkContainers(req.CreateNetworkContainerRequests) + response := cns.PostNetworkContainersResponse{ + Response: createNCsResp, + } + err = service.Listener.Encode(w, &response) + logger.Response(service.Name, response, response.Response.ReturnCode, err) +} + +func (service *HTTPRestService) createNetworkContainers(createNetworkContainerRequests []cns.CreateNetworkContainerRequest) cns.Response { returnCode := types.Success returnMessage := "" - for i := 0; i < len(req.CreateNetworkContainerRequests); i++ { - createNcReq := req.CreateNetworkContainerRequests[i] + + for i := 0; i < len(createNetworkContainerRequests); i++ { + createNcReq := createNetworkContainerRequests[i] ncDetails, found := service.getNetworkContainerDetails(createNcReq.NetworkContainerid) // Create NC if it doesn't exist, or it exists and the requested version is different from the saved version if !found || (found && ncDetails.VMVersion != createNcReq.Version) { nc := service.networkContainer - if err = nc.Create(createNcReq); err != nil { + if err := nc.Create(createNcReq); err != nil { returnCode = types.UnexpectedError returnMessage = fmt.Sprintf("[Azure CNS] Create Network Container failed with error: %s", err.Error()) break @@ -909,21 +919,17 @@ func (service *HTTPRestService) handlePostNetworkContainers(w http.ResponseWrite saveNcReturnCode, saveNcReturnMessage := service.saveNetworkContainerGoalState(createNcReq) // If NC was created successfully, log NC snapshot. - if saveNcReturnCode == types.Success { - logNCSnapshot(createNcReq) - } else { - returnCode = saveNcReturnCode - returnMessage = saveNcReturnMessage - break + if saveNcReturnCode != types.Success { + return cns.Response{ + ReturnCode: saveNcReturnCode, + Message: saveNcReturnMessage, + } } + logNCSnapshot(createNcReq) } - response := cns.PostNetworkContainersResponse{ - Response: cns.Response{ - ReturnCode: returnCode, - Message: returnMessage, - }, + return cns.Response{ + ReturnCode: returnCode, + Message: returnMessage, } - err = service.Listener.Encode(w, &response) - logger.Response(service.Name, response, response.Response.ReturnCode, err) } From 4ccdb526de9211f72f86ed4c62cbd66fae7878c9 Mon Sep 17 00:00:00 2001 From: bohuini Date: Thu, 15 Sep 2022 12:37:57 -0700 Subject: [PATCH 3/5] NC refresh optimization: Address comments --- cns/restserver/api.go | 2 -- cns/restserver/util.go | 5 +++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/cns/restserver/api.go b/cns/restserver/api.go index 6753285e5b..6688f4b0dc 100644 --- a/cns/restserver/api.go +++ b/cns/restserver/api.go @@ -883,8 +883,6 @@ func (service *HTTPRestService) getNetworkContainerByOrchestratorContext(w http. // If received "GET": Return all NCs in CNS's state file to DNC in order to check if NC refresh is needed // If received "POST": Store all the NCs (from the request body that client sent) into CNS's state file func (service *HTTPRestService) getOrRefreshNetworkContainers(w http.ResponseWriter, r *http.Request) { - logger.Printf("[Azure CNS] getOrRefreshNetworkContainers") - switch r.Method { case http.MethodGet: service.handleGetNetworkContainers(w) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index b91cdc623d..4a3c26922d 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -843,6 +843,7 @@ func (service *HTTPRestService) isNCWaitingForUpdate( // handleGetNetworkContainers returns all NCs in CNS func (service *HTTPRestService) handleGetNetworkContainers(w http.ResponseWriter) { + logger.Printf("[Azure CNS] getOrRefreshNetworkContainers do GET operation and return all NCs in CNS") service.RLock() networkContainers := make([]cns.GetNetworkContainerResponse, len(service.state.ContainerStatus)) i := 0 @@ -876,6 +877,7 @@ func (service *HTTPRestService) handleGetNetworkContainers(w http.ResponseWriter // handlePostNetworkContainers stores all the NCs (from the request that client sent) into CNS's state file func (service *HTTPRestService) handlePostNetworkContainers(w http.ResponseWriter, r *http.Request) { + logger.Printf("[Azure CNS] getOrRefreshNetworkContainers do POST operation and store all the NCs from the request into CNS") var req cns.PostNetworkContainersRequest err := service.Listener.Decode(w, r, &req) logger.Request(service.Name, &req, err) @@ -883,7 +885,7 @@ func (service *HTTPRestService) handlePostNetworkContainers(w http.ResponseWrite response := cns.PostNetworkContainersResponse{ Response: cns.Response{ ReturnCode: types.InvalidRequest, - Message: fmt.Sprintf("[Azure CNS] Create Network Container failed with error: %s", err.Error()), + Message: fmt.Sprintf("[Azure CNS] handlePostNetworkContainers failed with error: %s", err.Error()), }, } err = service.Listener.Encode(w, &response) @@ -917,7 +919,6 @@ func (service *HTTPRestService) createNetworkContainers(createNetworkContainerRe } // Save NC Goal State details saveNcReturnCode, saveNcReturnMessage := service.saveNetworkContainerGoalState(createNcReq) - // If NC was created successfully, log NC snapshot. if saveNcReturnCode != types.Success { return cns.Response{ From 76942db7a2dc2ecadb55444f4a9123ee9c2ec8be Mon Sep 17 00:00:00 2001 From: bohuini Date: Thu, 15 Sep 2022 12:56:05 -0700 Subject: [PATCH 4/5] NC refresh optimization: optimize createNetworkContainers() --- cns/restserver/util.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 4a3c26922d..04be005977 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -902,9 +902,6 @@ func (service *HTTPRestService) handlePostNetworkContainers(w http.ResponseWrite } func (service *HTTPRestService) createNetworkContainers(createNetworkContainerRequests []cns.CreateNetworkContainerRequest) cns.Response { - returnCode := types.Success - returnMessage := "" - for i := 0; i < len(createNetworkContainerRequests); i++ { createNcReq := createNetworkContainerRequests[i] ncDetails, found := service.getNetworkContainerDetails(createNcReq.NetworkContainerid) @@ -912,9 +909,10 @@ func (service *HTTPRestService) createNetworkContainers(createNetworkContainerRe if !found || (found && ncDetails.VMVersion != createNcReq.Version) { nc := service.networkContainer if err := nc.Create(createNcReq); err != nil { - returnCode = types.UnexpectedError - returnMessage = fmt.Sprintf("[Azure CNS] Create Network Container failed with error: %s", err.Error()) - break + return cns.Response{ + ReturnCode: types.UnexpectedError, + Message: fmt.Sprintf("[Azure CNS] Create Network Containers failed with error: %s", err.Error()), + } } } // Save NC Goal State details @@ -930,7 +928,7 @@ func (service *HTTPRestService) createNetworkContainers(createNetworkContainerRe } return cns.Response{ - ReturnCode: returnCode, - Message: returnMessage, + ReturnCode: types.Success, + Message: "", } } From c206c4751d3403bd7dc96d598be79f6d203de079 Mon Sep 17 00:00:00 2001 From: bohuini Date: Thu, 15 Sep 2022 14:08:58 -0700 Subject: [PATCH 5/5] NC refresh optimization: Fixed log info --- cns/restserver/util.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 04be005977..c85b76de0a 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -843,7 +843,7 @@ func (service *HTTPRestService) isNCWaitingForUpdate( // handleGetNetworkContainers returns all NCs in CNS func (service *HTTPRestService) handleGetNetworkContainers(w http.ResponseWriter) { - logger.Printf("[Azure CNS] getOrRefreshNetworkContainers do GET operation and return all NCs in CNS") + logger.Printf("[Azure CNS] handleGetNetworkContainers") service.RLock() networkContainers := make([]cns.GetNetworkContainerResponse, len(service.state.ContainerStatus)) i := 0 @@ -877,7 +877,7 @@ func (service *HTTPRestService) handleGetNetworkContainers(w http.ResponseWriter // handlePostNetworkContainers stores all the NCs (from the request that client sent) into CNS's state file func (service *HTTPRestService) handlePostNetworkContainers(w http.ResponseWriter, r *http.Request) { - logger.Printf("[Azure CNS] getOrRefreshNetworkContainers do POST operation and store all the NCs from the request into CNS") + logger.Printf("[Azure CNS] handlePostNetworkContainers") var req cns.PostNetworkContainersRequest err := service.Listener.Decode(w, r, &req) logger.Request(service.Name, &req, err)