From 6c57912ba13e0f63e112e1240c356d919f639f51 Mon Sep 17 00:00:00 2001 From: Ashvin Deodhar Date: Tue, 29 Sep 2020 22:40:58 -0700 Subject: [PATCH 1/9] Call NMA GetNCVersionStatus for non-managed scenario --- cns/nmagentclient/nmagentclient.go | 3 +- cns/restserver/api.go | 21 ++++++ cns/restserver/internalapi.go | 4 +- cns/restserver/restserver.go | 2 +- cns/restserver/util.go | 108 ++++++++++++++++++----------- 5 files changed, 94 insertions(+), 44 deletions(-) diff --git a/cns/nmagentclient/nmagentclient.go b/cns/nmagentclient/nmagentclient.go index 4b0d2d7790..bb770f2e16 100644 --- a/cns/nmagentclient/nmagentclient.go +++ b/cns/nmagentclient/nmagentclient.go @@ -10,7 +10,8 @@ import ( ) const ( - WireserverIP = "168.63.129.16" + WireserverIP = "168.63.129.16" + GetNetworkContainerVersionURLFmt = "http://%s/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/%s/networkContainers/%s/version/authenticationToken/%s/api-version/1" ) // NMANetworkContainerResponse - NMAgent response. diff --git a/cns/restserver/api.go b/cns/restserver/api.go index 9a3ea73587..e38a10cdb7 100644 --- a/cns/restserver/api.go +++ b/cns/restserver/api.go @@ -1128,6 +1128,16 @@ func (service *HTTPRestService) getNumberOfCPUCores(w http.ResponseWriter, r *ht logger.Response(service.Name, numOfCPUCoresResp, resp.ReturnCode, ReturnCodeToString(resp.ReturnCode), err) } +func getInterfaceIdFromCreateNetworkContainerURL( + createNetworkContainerURL string) string { + return strings.Split(strings.Split(createNetworkContainerURL, "interfaces/")[1], "/")[0] +} + +func getAuthTokenFromCreateNetworkContainerURL( + createNetworkContainerURL string) string { + return strings.Split(strings.Split(createNetworkContainerURL, "authenticationToken/")[1], "/")[0] +} + // Publish Network Container by calling nmagent func (service *HTTPRestService) publishNetworkContainer(w http.ResponseWriter, r *http.Request) { logger.Printf("[Azure-CNS] PublishNetworkContainer") @@ -1184,6 +1194,17 @@ func (service *HTTPRestService) publishNetworkContainer(w http.ResponseWriter, r logger.Errorf("[Azure-CNS] %s", returnMessage) } } + + // Store ncGetVersionURL needed for calling NMAgent to check if vfp programming is completed for the NC + primaryInterfaceIdentifier := getInterfaceIdFromCreateNetworkContainerURL(req.CreateNetworkContainerURL) + authToken := getAuthTokenFromCreateNetworkContainerURL(req.CreateNetworkContainerURL) + ncGetVersionURL := fmt.Sprintf(nmagentclient.GetNetworkContainerVersionURLFmt, + nmagentclient.WireserverIP, + primaryInterfaceIdentifier, + req.NetworkContainerID, + authToken) + ncVersionURLs.Store(cns.SwiftPrefix+req.NetworkContainerID, ncGetVersionURL) + default: returnMessage = "PublishNetworkContainer API expects a POST" returnCode = UnsupportedVerb diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index a15915e15e..3d0083a2c4 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -103,7 +103,7 @@ func (service *HTTPRestService) SyncNodeStatus(dncEP, infraVnet, nodeID string, ) ncVersionURLs.Store(nc.NetworkContainerid, versionURL) - waitingForUpdate, tmpReturnCode, tmpErrStr := isNCWaitingForUpdate(nc.Version, nc.NetworkContainerid) + waitingForUpdate, tmpReturnCode, tmpErrStr := service.isNCWaitingForUpdate(nc.Version, nc.NetworkContainerid) if tmpReturnCode != Success && bytes.Compare(nc.OrchestratorContext, contextFromCNI) == 0 { returnCode = tmpReturnCode errStr = tmpErrStr @@ -122,7 +122,7 @@ func (service *HTTPRestService) SyncNodeStatus(dncEP, infraVnet, nodeID string, if err = json.Unmarshal(w.Body.Bytes(), &resp); err == nil && resp.Response.ReturnCode == Success { service.Lock() ncstatus, _ := service.state.ContainerStatus[ncid] - ncstatus.WaitingForUpdate = waitingForUpdate + ncstatus.VfpUpdateComplete = !waitingForUpdate service.state.ContainerStatus[ncid] = ncstatus service.Unlock() } diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index 17e2d050aa..b8d0df3165 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -60,7 +60,7 @@ type containerstatus struct { VMVersion string HostVersion string CreateNetworkContainerRequest cns.CreateNetworkContainerRequest - WaitingForUpdate bool // True when NC is waiting for NMA to sync versions/rules + VfpUpdateComplete bool // True when VFP programming is completed for the NC } // httpRestServiceState contains the state we would like to persist. diff --git a/cns/restserver/util.go b/cns/restserver/util.go index b493bd160a..4a9a10ef9b 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -105,16 +105,21 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw service.Lock() defer service.Unlock() + var ( + hostVersion string + existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig //uuid is key + vfpUpdateComplete bool + ) + if service.state.ContainerStatus == nil { service.state.ContainerStatus = make(map[string]containerstatus) } existingNCStatus, ok := service.state.ContainerStatus[req.NetworkContainerid] - var hostVersion string - var existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig //uuid is key if ok { hostVersion = existingNCStatus.HostVersion existingSecondaryIPConfigs = existingNCStatus.CreateNetworkContainerRequest.SecondaryIPConfigs + vfpUpdateComplete = existingNCStatus.VfpUpdateComplete } service.state.ContainerStatus[req.NetworkContainerid] = @@ -122,7 +127,8 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw ID: req.NetworkContainerid, VMVersion: req.Version, CreateNetworkContainerRequest: req, - HostVersion: hostVersion} + HostVersion: hostVersion, + VfpUpdateComplete: vfpUpdateComplete} switch req.NetworkContainerType { case cns.AzureContainerInstance: @@ -182,13 +188,6 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw return UnsupportedNetworkContainerType, errMsg } - service.state.ContainerStatus[req.NetworkContainerid] = - containerstatus{ - ID: req.NetworkContainerid, - VMVersion: req.Version, - CreateNetworkContainerRequest: req, - HostVersion: hostVersion} - service.saveState() return 0, "" } @@ -305,10 +304,11 @@ func (service *HTTPRestService) getNetworkContainerResponse(req cns.GetNetworkCo containerID string getNetworkContainerResponse cns.GetNetworkContainerResponse exists bool + waitingForUpdate bool ) - service.RLock() - defer service.RUnlock() + service.Lock() + defer service.Unlock() switch service.state.OrchestratorType { case cns.Kubernetes: @@ -332,28 +332,44 @@ func (service *HTTPRestService) getNetworkContainerResponse(req cns.GetNetworkCo context := podInfo.PodName + podInfo.PodNamespace containerID, exists = service.state.ContainerIDByOrchestratorContext[context] - if service.ChannelMode == cns.Managed { - if exists { - _, getNetworkContainerResponse.Response.ReturnCode, getNetworkContainerResponse.Response.Message = isNCWaitingForUpdate(service.state.ContainerStatus[containerID].CreateNetworkContainerRequest.Version, containerID) - if getNetworkContainerResponse.Response.ReturnCode == Success { - return getNetworkContainerResponse - } - } else { - var ( - dncEP = service.GetOption(acn.OptPrivateEndpoint).(string) - infraVnet = service.GetOption(acn.OptInfrastructureNetworkID).(string) - nodeID = service.GetOption(acn.OptNodeID).(string) - ) - - service.RUnlock() - getNetworkContainerResponse.Response.ReturnCode, getNetworkContainerResponse.Response.Message = service.SyncNodeStatus(dncEP, infraVnet, nodeID, req.OrchestratorContext) - service.RLock() - if getNetworkContainerResponse.Response.ReturnCode == NotFound { - return getNetworkContainerResponse - } - containerID = service.state.ContainerIDByOrchestratorContext[context] + if exists { + // If the goal state is available with CNS, check if the NC is pending VFP programming + waitingForUpdate, getNetworkContainerResponse.Response.ReturnCode, getNetworkContainerResponse.Response.Message = + service.isNCWaitingForUpdate(service.state.ContainerStatus[containerID].CreateNetworkContainerRequest.Version, containerID) + // If the return code is not success, return the error to the caller + if getNetworkContainerResponse.Response.ReturnCode != Success { + logger.Errorf("[Azure-CNS] isNCWaitingForUpdate failed for NC: %s with error: %s", + containerID, getNetworkContainerResponse.Response.Message) + return getNetworkContainerResponse } + + vfpUpdateComplete := !waitingForUpdate + ncstatus, _ := service.state.ContainerStatus[containerID] + // Update the container status if needed + if ncstatus.VfpUpdateComplete != vfpUpdateComplete { + logger.Printf("[Azure-CNS] Setting VfpUpdateComplete to %t for NC: %s", vfpUpdateComplete, containerID) + ncstatus.VfpUpdateComplete = vfpUpdateComplete + service.state.ContainerStatus[containerID] = ncstatus + service.saveState() + } + + } else if service.ChannelMode == cns.Managed { + // If the NC goal state doesn't exist in CNS running in managed mode, call DNC to retrieve the goal state + var ( + dncEP = service.GetOption(acn.OptPrivateEndpoint).(string) + infraVnet = service.GetOption(acn.OptInfrastructureNetworkID).(string) + nodeID = service.GetOption(acn.OptNodeID).(string) + ) + + service.Unlock() + getNetworkContainerResponse.Response.ReturnCode, getNetworkContainerResponse.Response.Message = service.SyncNodeStatus(dncEP, infraVnet, nodeID, req.OrchestratorContext) + service.Lock() + if getNetworkContainerResponse.Response.ReturnCode == NotFound { + return getNetworkContainerResponse + } + + containerID = service.state.ContainerIDByOrchestratorContext[context] } logger.Printf("containerid %v", containerID) @@ -456,8 +472,8 @@ func (service *HTTPRestService) attachOrDetachHelper(req cns.ConfigureContainerN existing, ok := service.getNetworkContainerDetails(cns.SwiftPrefix + req.NetworkContainerid) if service.ChannelMode == cns.Managed && operation == attach { if ok { - if existing.WaitingForUpdate { - _, returnCode, message := isNCWaitingForUpdate(existing.CreateNetworkContainerRequest.Version, req.NetworkContainerid) + if !existing.VfpUpdateComplete { + _, returnCode, message := service.isNCWaitingForUpdate(existing.CreateNetworkContainerRequest.Version, req.NetworkContainerid) if returnCode != Success { return cns.Response{ ReturnCode: returnCode, @@ -673,11 +689,23 @@ func (service *HTTPRestService) populateIpConfigInfoUntransacted(ipConfigStatus } // isNCWaitingForUpdate :- Determine whether NC version on NMA matches programmed version -func isNCWaitingForUpdate(ncVersion, ncid string) (waitingForUpdate bool, returnCode int, message string) { +func (service *HTTPRestService) isNCWaitingForUpdate(ncVersion, ncid string) (waitingForUpdate bool, returnCode int, message string) { + ncStatus, ok := service.state.ContainerStatus[ncid] + if ok { + if ncStatus.VfpUpdateComplete && + (ncStatus.CreateNetworkContainerRequest.Version == ncVersion) { + waitingForUpdate = false + returnCode = Success + message = fmt.Sprintf("Network container: %s, version: %s has VFP programming already completed", ncid, ncVersion) + logger.Printf("[Azure CNS] %s", message) + return + } + } + getNCVersionURL, ok := ncVersionURLs.Load(ncid) if !ok { returnCode = NotFound - message = fmt.Sprintf("[Azure-CNS] Network container %s not found", ncid) + message = fmt.Sprintf("[Azure-CNS] getNCVersionURL for Network container %s not found", ncid) return } @@ -693,19 +721,19 @@ func isNCWaitingForUpdate(ncVersion, ncid string) (waitingForUpdate bool, return if programmedVersion > nmaVersion { waitingForUpdate = true returnCode = NetworkContainerPendingStatePropagation - message = fmt.Sprintf("[Azure-CNS] Network container %s v%d had not propagated to respective NMA w/ v%d", ncid, programmedVersion, nmaVersion) + message = fmt.Sprintf("Network container %s v%d had not propagated to respective NMA w/ v%d", ncid, programmedVersion, nmaVersion) } } else { returnCode = UnexpectedError - message = fmt.Sprintf("[Azure-CNS] Failed to get NC version from response %s for NC %s", rBytes, ncid) + message = fmt.Sprintf("Failed to get NC version status from NMAgent. NC: %s, Response %s", ncid, rBytes) } } else { returnCode = UnexpectedError - message = fmt.Sprintf("[Azure-CNS] Failed to get NC version with http status %d", response.StatusCode) + message = fmt.Sprintf("Failed to get NC version status from NMAgent with http status %d", response.StatusCode) } } else { returnCode = UnexpectedError - message = fmt.Sprintf("[Azure-CNS] Failed to get NC version from NMA with error: %+v", err) + message = fmt.Sprintf("Failed to get NC version status from NMAgent with error: %+v", err) } return From 93ff54d8e10a67d97d35d11c951e9b633f6338a1 Mon Sep 17 00:00:00 2001 From: Ashvin Deodhar Date: Mon, 5 Oct 2020 10:20:38 -0700 Subject: [PATCH 2/9] Update --- cns/restserver/const.go | 64 +++++++++++++------------- cns/restserver/internalapi.go | 10 +--- cns/restserver/util.go | 87 +++++++++++++++++++++-------------- 3 files changed, 87 insertions(+), 74 deletions(-) diff --git a/cns/restserver/const.go b/cns/restserver/const.go index 0beda46f19..b7ea9838df 100644 --- a/cns/restserver/const.go +++ b/cns/restserver/const.go @@ -5,37 +5,39 @@ package restserver // Container Network Service remote API Contract. const ( - Success = 0 - UnsupportedNetworkType = 1 - InvalidParameter = 2 - UnsupportedEnvironment = 3 - UnreachableHost = 4 - ReservationNotFound = 5 - MalformedSubnet = 8 - UnreachableDockerDaemon = 9 - UnspecifiedNetworkName = 10 - NotFound = 14 - AddressUnavailable = 15 - NetworkContainerNotSpecified = 16 - CallToHostFailed = 17 - UnknownContainerID = 18 - UnsupportedOrchestratorType = 19 - DockerContainerNotSpecified = 20 - UnsupportedVerb = 21 - UnsupportedNetworkContainerType = 22 - InvalidRequest = 23 - NetworkJoinFailed = 24 - NetworkContainerPublishFailed = 25 - NetworkContainerUnpublishFailed = 26 - InvalidPrimaryIPConfig = 27 - PrimaryCANotSame = 28 - InconsistentIPConfigState = 29 - InvalidSecondaryIPConfig = 30 - NetworkContainerPendingStatePropagation = 31 - FailedToAllocateIpConfig = 32 - EmptyOrchestratorContext = 33 - UnsupportedOrchestratorContext = 34 - UnexpectedError = 99 + Success = 0 + UnsupportedNetworkType = 1 + InvalidParameter = 2 + UnsupportedEnvironment = 3 + UnreachableHost = 4 + ReservationNotFound = 5 + MalformedSubnet = 8 + UnreachableDockerDaemon = 9 + UnspecifiedNetworkName = 10 + NotFound = 14 + AddressUnavailable = 15 + NetworkContainerNotSpecified = 16 + CallToHostFailed = 17 + UnknownContainerID = 18 + UnsupportedOrchestratorType = 19 + DockerContainerNotSpecified = 20 + UnsupportedVerb = 21 + UnsupportedNetworkContainerType = 22 + InvalidRequest = 23 + NetworkJoinFailed = 24 + NetworkContainerPublishFailed = 25 + NetworkContainerUnpublishFailed = 26 + InvalidPrimaryIPConfig = 27 + PrimaryCANotSame = 28 + InconsistentIPConfigState = 29 + InvalidSecondaryIPConfig = 30 + NetworkContainerVfpProgramPending = 31 + FailedToAllocateIpConfig = 32 + EmptyOrchestratorContext = 33 + UnsupportedOrchestratorContext = 34 + NetworkContainerVfpProgramComplete = 35 + NetworkContainerVfpProgramCheckSkipped = 36 + UnexpectedError = 99 ) const ( diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 3d0083a2c4..6cc2812a27 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -103,15 +103,7 @@ func (service *HTTPRestService) SyncNodeStatus(dncEP, infraVnet, nodeID string, ) ncVersionURLs.Store(nc.NetworkContainerid, versionURL) - waitingForUpdate, tmpReturnCode, tmpErrStr := service.isNCWaitingForUpdate(nc.Version, nc.NetworkContainerid) - if tmpReturnCode != Success && bytes.Compare(nc.OrchestratorContext, contextFromCNI) == 0 { - returnCode = tmpReturnCode - errStr = tmpErrStr - } - - if tmpReturnCode == UnexpectedError { - continue - } + waitingForUpdate, _, _ := service.isNCWaitingForUpdate(nc.Version, nc.NetworkContainerid) body, _ = json.Marshal(nc) req, _ = http.NewRequest(http.MethodPost, "", bytes.NewBuffer(body)) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 4a9a10ef9b..a5f4e5dad0 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -338,7 +338,7 @@ func (service *HTTPRestService) getNetworkContainerResponse(req cns.GetNetworkCo waitingForUpdate, getNetworkContainerResponse.Response.ReturnCode, getNetworkContainerResponse.Response.Message = service.isNCWaitingForUpdate(service.state.ContainerStatus[containerID].CreateNetworkContainerRequest.Version, containerID) // If the return code is not success, return the error to the caller - if getNetworkContainerResponse.Response.ReturnCode != Success { + if getNetworkContainerResponse.Response.ReturnCode == NetworkContainerVfpProgramPending { logger.Errorf("[Azure-CNS] isNCWaitingForUpdate failed for NC: %s with error: %s", containerID, getNetworkContainerResponse.Response.Message) return getNetworkContainerResponse @@ -346,8 +346,12 @@ func (service *HTTPRestService) getNetworkContainerResponse(req cns.GetNetworkCo vfpUpdateComplete := !waitingForUpdate ncstatus, _ := service.state.ContainerStatus[containerID] - // Update the container status if needed - if ncstatus.VfpUpdateComplete != vfpUpdateComplete { + // Update the container status if- + // 1. VfpUpdateCompleted successfully + // 2. VfpUpdateComplete changed to false + if (getNetworkContainerResponse.Response.ReturnCode == NetworkContainerVfpProgramComplete && + vfpUpdateComplete == true && ncstatus.VfpUpdateComplete != vfpUpdateComplete) || + (vfpUpdateComplete == false && ncstatus.VfpUpdateComplete != vfpUpdateComplete) { logger.Printf("[Azure-CNS] Setting VfpUpdateComplete to %t for NC: %s", vfpUpdateComplete, containerID) ncstatus.VfpUpdateComplete = vfpUpdateComplete service.state.ContainerStatus[containerID] = ncstatus @@ -474,7 +478,7 @@ func (service *HTTPRestService) attachOrDetachHelper(req cns.ConfigureContainerN if ok { if !existing.VfpUpdateComplete { _, returnCode, message := service.isNCWaitingForUpdate(existing.CreateNetworkContainerRequest.Version, req.NetworkContainerid) - if returnCode != Success { + if returnCode == NetworkContainerVfpProgramPending { return cns.Response{ ReturnCode: returnCode, Message: message} @@ -689,51 +693,66 @@ func (service *HTTPRestService) populateIpConfigInfoUntransacted(ipConfigStatus } // isNCWaitingForUpdate :- Determine whether NC version on NMA matches programmed version +// Return error and waitingForUpdate as true only CNS gets response from NMAgent indicating the VFP programming is pending +// This returns success / waitingForUpdate as false in all other cases. func (service *HTTPRestService) isNCWaitingForUpdate(ncVersion, ncid string) (waitingForUpdate bool, returnCode int, message string) { + waitingForUpdate = true ncStatus, ok := service.state.ContainerStatus[ncid] if ok { if ncStatus.VfpUpdateComplete && (ncStatus.CreateNetworkContainerRequest.Version == ncVersion) { + logger.Printf("[Azure CNS] Network container: %s, version: %s has VFP programming already completed", ncid, ncVersion) + returnCode = NetworkContainerVfpProgramCheckSkipped waitingForUpdate = false - returnCode = Success - message = fmt.Sprintf("Network container: %s, version: %s has VFP programming already completed", ncid, ncVersion) - logger.Printf("[Azure CNS] %s", message) return } } getNCVersionURL, ok := ncVersionURLs.Load(ncid) if !ok { - returnCode = NotFound - message = fmt.Sprintf("[Azure-CNS] getNCVersionURL for Network container %s not found", ncid) + logger.Printf("[Azure CNS] getNCVersionURL for Network container %s not found. Skipping GetNCVersionStatus check from NMAgent", + ncid) + returnCode = NetworkContainerVfpProgramCheckSkipped return } response, err := nmagentclient.GetNetworkContainerVersion(ncid, getNCVersionURL.(string)) - if err == nil { - if response.StatusCode == http.StatusOK { - var versionResponse nmagentclient.NMANetworkContainerResponse - rBytes, _ := ioutil.ReadAll(response.Body) - json.Unmarshal(rBytes, &versionResponse) - if versionResponse.ResponseCode == "200" { - programmedVersion, _ := strconv.Atoi(ncVersion) - nmaVersion, _ := strconv.Atoi(versionResponse.Version) - if programmedVersion > nmaVersion { - waitingForUpdate = true - returnCode = NetworkContainerPendingStatePropagation - message = fmt.Sprintf("Network container %s v%d had not propagated to respective NMA w/ v%d", ncid, programmedVersion, nmaVersion) - } - } else { - returnCode = UnexpectedError - message = fmt.Sprintf("Failed to get NC version status from NMAgent. NC: %s, Response %s", ncid, rBytes) - } - } else { - returnCode = UnexpectedError - message = fmt.Sprintf("Failed to get NC version status from NMAgent with http status %d", response.StatusCode) - } + if err != nil { + logger.Printf("[Azure CNS] Failed to get NC version status from NMAgent with error: %+v. "+ + "Skipping GetNCVersionStatus check from NMAgent", err) + returnCode = NetworkContainerVfpProgramCheckSkipped + return + } + + if response.StatusCode != http.StatusOK { + logger.Printf("[Azure CNS] Failed to get NC version status from NMAgent with http status %d. "+ + "Skipping GetNCVersionStatus check from NMAgent", response.StatusCode) + returnCode = NetworkContainerVfpProgramCheckSkipped + return + } + + var versionResponse nmagentclient.NMANetworkContainerResponse + rBytes, _ := ioutil.ReadAll(response.Body) + json.Unmarshal(rBytes, &versionResponse) + if versionResponse.ResponseCode != "200" { + returnCode = NetworkContainerVfpProgramPending + message = fmt.Sprintf("Failed to get NC version status from NMAgent. NC: %s, Response %s", ncid, rBytes) + return + } + + ncTargetVersion, _ := strconv.Atoi(ncVersion) + nmaProgrammedNCVersion, _ := strconv.Atoi(versionResponse.Version) + logger.Printf("[Azure CNS] tempdebug: 1 %d %s", ncTargetVersion, ncVersion) + logger.Printf("[Azure CNS] tempdebug: 2 %d %s", nmaProgrammedNCVersion, versionResponse.Version) + if ncTargetVersion > nmaProgrammedNCVersion { + returnCode = NetworkContainerVfpProgramPending + message = fmt.Sprintf("Network container: %s version: %d is not yet programmed by NMAgent. Programmed version: %d", + ncid, ncTargetVersion, nmaProgrammedNCVersion) } else { - returnCode = UnexpectedError - message = fmt.Sprintf("Failed to get NC version status from NMAgent with error: %+v", err) + returnCode = NetworkContainerVfpProgramComplete + waitingForUpdate = false + message = fmt.Sprintf("Vfp programming complete") + logger.Printf("[Azure CNS] Vfp programming complete for NC: %s", ncid) } return @@ -774,8 +793,8 @@ func ReturnCodeToString(returnCode int) (s string) { s = "UnexpectedError" case DockerContainerNotSpecified: s = "DockerContainerNotSpecified" - case NetworkContainerPendingStatePropagation: - s = "NetworkContainerPendingStatePropagation" + case NetworkContainerVfpProgramPending: + s = "NetworkContainerVfpProgramPending" default: s = "UnknownError" } From d27c7941d07caf9974f1401dd16dbf81833e9cae Mon Sep 17 00:00:00 2001 From: Ashvin Deodhar Date: Mon, 5 Oct 2020 11:34:14 -0700 Subject: [PATCH 3/9] remove debug logs --- cns/restserver/util.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index a5f4e5dad0..b13dbfb4f2 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -742,8 +742,6 @@ func (service *HTTPRestService) isNCWaitingForUpdate(ncVersion, ncid string) (wa ncTargetVersion, _ := strconv.Atoi(ncVersion) nmaProgrammedNCVersion, _ := strconv.Atoi(versionResponse.Version) - logger.Printf("[Azure CNS] tempdebug: 1 %d %s", ncTargetVersion, ncVersion) - logger.Printf("[Azure CNS] tempdebug: 2 %d %s", nmaProgrammedNCVersion, versionResponse.Version) if ncTargetVersion > nmaProgrammedNCVersion { returnCode = NetworkContainerVfpProgramPending message = fmt.Sprintf("Network container: %s version: %d is not yet programmed by NMAgent. Programmed version: %d", @@ -752,7 +750,7 @@ func (service *HTTPRestService) isNCWaitingForUpdate(ncVersion, ncid string) (wa returnCode = NetworkContainerVfpProgramComplete waitingForUpdate = false message = fmt.Sprintf("Vfp programming complete") - logger.Printf("[Azure CNS] Vfp programming complete for NC: %s", ncid) + logger.Printf("[Azure CNS] Vfp programming complete for NC: %s with version: %d", ncid, ncTargetVersion) } return From 3ac011878923262f0c56a0dc76afb919d5a04408 Mon Sep 17 00:00:00 2001 From: Ashvin Deodhar Date: Mon, 5 Oct 2020 12:40:14 -0700 Subject: [PATCH 4/9] update test --- cns/restserver/api_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 875d32d1ee..7b0e246141 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -350,7 +350,8 @@ func TestPublishNCViaCNS(t *testing.T) { networkID := "vnet1" networkContainerID := "ethWebApp" joinNetworkURL := "http://" + nmagentEndpoint + "/dummyVnetURL" - createNetworkContainerURL := "http://" + nmagentEndpoint + "/networkContainers/dummyNCURL" + createNetworkContainerURL := "http://" + nmagentEndpoint + + "interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version" publishNCRequest := &cns.PublishNetworkContainerRequest{ NetworkID: networkID, From 1969dd15e59d288307e1dd615ebdff1db9c581c3 Mon Sep 17 00:00:00 2001 From: Ashvin Deodhar Date: Mon, 5 Oct 2020 12:47:41 -0700 Subject: [PATCH 5/9] Fix test --- cns/restserver/api_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 7b0e246141..1bc0ace751 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -351,7 +351,7 @@ func TestPublishNCViaCNS(t *testing.T) { networkContainerID := "ethWebApp" joinNetworkURL := "http://" + nmagentEndpoint + "/dummyVnetURL" createNetworkContainerURL := "http://" + nmagentEndpoint + - "interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version" + "/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version" publishNCRequest := &cns.PublishNetworkContainerRequest{ NetworkID: networkID, From be6d0ec56a14d9a42fc6d2ab30012e91b446befe Mon Sep 17 00:00:00 2001 From: Ashvin Deodhar Date: Mon, 5 Oct 2020 22:49:05 -0700 Subject: [PATCH 6/9] Add test coverage --- cns/nmagentclient/nmagentclient.go | 3 +- cns/restserver/api_test.go | 170 ++++++++++++++++++++++++++--- 2 files changed, 156 insertions(+), 17 deletions(-) diff --git a/cns/nmagentclient/nmagentclient.go b/cns/nmagentclient/nmagentclient.go index bb770f2e16..567ca06cc1 100644 --- a/cns/nmagentclient/nmagentclient.go +++ b/cns/nmagentclient/nmagentclient.go @@ -10,10 +10,11 @@ import ( ) const ( - WireserverIP = "168.63.129.16" GetNetworkContainerVersionURLFmt = "http://%s/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/%s/networkContainers/%s/version/authenticationToken/%s/api-version/1" ) +var WireserverIP = "168.63.129.16" + // NMANetworkContainerResponse - NMAgent response. type NMANetworkContainerResponse struct { ResponseCode string `json:"httpStatusCode"` diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 1bc0ace751..70801b9233 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -19,6 +19,7 @@ import ( "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/fakes" "github.com/Azure/azure-container-networking/cns/logger" + "github.com/Azure/azure-container-networking/cns/nmagentclient" acncommon "github.com/Azure/azure-container-networking/common" ) @@ -54,7 +55,7 @@ var ( service cns.HTTPService svc *HTTPRestService mux *http.ServeMux - hostQueryForProgrammedVersionResponse = `{"httpStatusCode":"200","networkContainerId":"eab2470f-test-test-test-b3cd316979d5","version":"1"}` + hostQueryForProgrammedVersionResponse = `{"httpStatusCode":"200","networkContainerId":"ethWebApp","version":"0"}` hostQueryResponse = xmlDocument{ XMLName: xml.Name{Local: "Interfaces"}, Interface: []Interface{Interface{ @@ -79,6 +80,13 @@ const ( nmagentEndpoint = "localhost:9000" ) +type createOrUpdateNetworkContainerParams struct { + ncID string + ncIP string + ncType string + ncVersion string +} + func getInterfaceInfo(w http.ResponseWriter, r *http.Request) { w.Header().Set(acncommon.ContentType, "application/xml") output, _ := xml.Marshal(hostQueryResponse) @@ -115,6 +123,7 @@ func TestMain(m *testing.M) { nmAgentServer.AddHandler("/getInterface", getInterfaceInfo) nmAgentServer.AddHandler("/", nmagentHandler) + nmagentclient.WireserverIP = nmagentEndpoint err = nmAgentServer.Start(make(chan error, 1)) if err != nil { @@ -167,7 +176,15 @@ func TestCreateNetworkContainer(t *testing.T) { // Test create network container of type JobObject fmt.Println("TestCreateNetworkContainer: JobObject") - err := creatOrUpdateNetworkContainerWithName(t, "testJobObject", "10.1.0.5", "JobObject") + + params := createOrUpdateNetworkContainerParams{ + ncID: "testJobObject", + ncIP: "10.1.0.5", + ncType: "JobObject", + ncVersion: "0", + } + + err := createOrUpdateNetworkContainerWithParams(t, params) if err != nil { t.Errorf("Failed to save the goal state for network container of type JobObject "+ " due to error: %+v", err) @@ -183,13 +200,27 @@ func TestCreateNetworkContainer(t *testing.T) { // Test create network container of type WebApps fmt.Println("TestCreateNetworkContainer: WebApps") - err = creatOrUpdateNetworkContainerWithName(t, "ethWebApp", "192.0.0.5", "WebApps") + params = createOrUpdateNetworkContainerParams{ + ncID: "ethWebApp", + ncIP: "192.0.0.5", + ncType: "WebApps", + ncVersion: "0", + } + + err = createOrUpdateNetworkContainerWithParams(t, params) if err != nil { t.Errorf("creatOrUpdateWebAppContainerWithName failed Err:%+v", err) t.Fatal(err) } - err = creatOrUpdateNetworkContainerWithName(t, "ethWebApp", "192.0.0.6", "WebApps") + params = createOrUpdateNetworkContainerParams{ + ncID: "ethWebApp", + ncIP: "192.0.0.6", + ncType: "WebApps", + ncVersion: "0", + } + + err = createOrUpdateNetworkContainerWithParams(t, params) if err != nil { t.Errorf("Updating interface failed Err:%+v", err) t.Fatal(err) @@ -204,7 +235,14 @@ func TestCreateNetworkContainer(t *testing.T) { } // Test create network container of type COW - err = creatOrUpdateNetworkContainerWithName(t, "testCOWContainer", "10.0.0.5", "COW") + params = createOrUpdateNetworkContainerParams{ + ncID: "testCOWContainer", + ncIP: "10.0.0.5", + ncType: "COW", + ncVersion: "0", + } + + err = createOrUpdateNetworkContainerWithParams(t, params) if err != nil { t.Errorf("Failed to save the goal state for network container of type COW"+ " due to error: %+v", err) @@ -227,9 +265,16 @@ func TestGetNetworkContainerByOrchestratorContext(t *testing.T) { setEnv(t) setOrchestratorType(t, cns.Kubernetes) - err := creatOrUpdateNetworkContainerWithName(t, "ethWebApp", "11.0.0.5", cns.AzureContainerInstance) + params := createOrUpdateNetworkContainerParams{ + ncID: "ethWebApp", + ncIP: "11.0.0.5", + ncType: cns.AzureContainerInstance, + ncVersion: "0", + } + + err := createOrUpdateNetworkContainerWithParams(t, params) if err != nil { - t.Errorf("creatOrUpdateNetworkContainerWithName failed Err:%+v", err) + t.Errorf("createOrUpdateNetworkContainerWithParams failed Err:%+v", err) t.Fatal(err) } @@ -262,7 +307,7 @@ func TestGetNetworkContainerByOrchestratorContext(t *testing.T) { // setEnv(t) // setOrchestratorType(t, cns.Kubernetes) -// err := creatOrUpdateNetworkContainerWithName(t, "ethWebApp", "11.0.0.5", cns.AzureContainerInstance) +// err := createOrUpdateNetworkContainerWithParams(t, "ethWebApp", "11.0.0.5", cns.AzureContainerInstance) // if err != nil { // t.Errorf("creatOrUpdateWebAppContainerWithName failed Err:%+v", err) // t.Fatal(err) @@ -291,7 +336,14 @@ func TestGetInterfaceForNetworkContainer(t *testing.T) { setEnv(t) setOrchestratorType(t, cns.Kubernetes) - err := creatOrUpdateNetworkContainerWithName(t, "ethWebApp", "11.0.0.5", "WebApps") + params := createOrUpdateNetworkContainerParams{ + ncID: "ethWebApp", + ncIP: "11.0.0.5", + ncType: "WebApps", + ncVersion: "0", + } + + err := createOrUpdateNetworkContainerWithParams(t, params) if err != nil { t.Errorf("creatOrUpdateWebAppContainerWithName failed Err:%+v", err) t.Fatal(err) @@ -339,16 +391,75 @@ func TestGetNumOfCPUCores(t *testing.T) { } } +func TestGetNetworkContainerVersionStatus(t *testing.T) { + fmt.Println("Test: TestGetNetworkContainerVersionStatus") + + setEnv(t) + setOrchestratorType(t, cns.Kubernetes) + + params := createOrUpdateNetworkContainerParams{ + ncID: "ethWebApp", + ncIP: "11.0.0.5", + ncType: cns.AzureContainerInstance, + ncVersion: "0", + } + + if err := createOrUpdateNetworkContainerWithParams(t, params); err != nil { + t.Errorf("createOrUpdateNetworkContainerWithParams failed Err:%+v", err) + t.Fatal(err) + } + + publishNCViaCNS(t, "vnet1", "ethWebApp") + + if err := getNetworkContainerByContext(t, "ethWebApp"); err != nil { + t.Errorf("TestGetNetworkContainerByOrchestratorContext failed Err:%+v", err) + t.Fatal(err) + } + + if err := deleteNetworkAdapterWithName(t, "ethWebApp"); err != nil { + t.Errorf("Deleting interface failed Err:%+v", err) + t.Fatal(err) + } + + params = createOrUpdateNetworkContainerParams{ + ncID: "ethWebAppUpdated", + ncIP: "11.0.0.5", + ncType: cns.AzureContainerInstance, + ncVersion: "1", + } + + if err := createOrUpdateNetworkContainerWithParams(t, params); err != nil { + t.Errorf("createOrUpdateNetworkContainerWithParams failed Err:%+v", err) + t.Fatal(err) + } + + publishNCViaCNS(t, "vnet1", "ethWebAppUpdated") + + if err := getNetworkContainerByContextExpectedError(t, "ethWebAppUpdated"); err != nil { + t.Errorf("TestGetNetworkContainerVersionStatus failed") + t.Fatal(err) + } + + if err := deleteNetworkAdapterWithName(t, "ethWebAppUpdated"); err != nil { + t.Errorf("Deleting interface failed Err:%+v", err) + t.Fatal(err) + } +} + func TestPublishNCViaCNS(t *testing.T) { fmt.Println("Test: publishNetworkContainer") + publishNCViaCNS(t, "vnet1", "ethWebApp") +} + +func publishNCViaCNS(t *testing.T, + networkID string, + networkContainerID string) { var ( body bytes.Buffer resp cns.PublishNetworkContainerResponse ) - networkID := "vnet1" - networkContainerID := "ethWebApp" joinNetworkURL := "http://" + nmagentEndpoint + "/dummyVnetURL" createNetworkContainerURL := "http://" + nmagentEndpoint + "/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version" @@ -446,22 +557,22 @@ func setOrchestratorType(t *testing.T, orchestratorType string) error { return nil } -func creatOrUpdateNetworkContainerWithName(t *testing.T, name string, ip string, containerType string) error { +func createOrUpdateNetworkContainerWithParams(t *testing.T, params createOrUpdateNetworkContainerParams) error { var body bytes.Buffer var ipConfig cns.IPConfiguration ipConfig.DNSServers = []string{"8.8.8.8", "8.8.4.4"} ipConfig.GatewayIPAddress = "11.0.0.1" var ipSubnet cns.IPSubnet - ipSubnet.IPAddress = ip + ipSubnet.IPAddress = params.ncIP ipSubnet.PrefixLength = 24 ipConfig.IPSubnet = ipSubnet podInfo := cns.KubernetesPodInfo{PodName: "testpod", PodNamespace: "testpodnamespace"} context, _ := json.Marshal(podInfo) info := &cns.CreateNetworkContainerRequest{ - Version: "0.1", - NetworkContainerType: containerType, - NetworkContainerid: name, + Version: params.ncVersion, + NetworkContainerType: params.ncType, + NetworkContainerid: cns.SwiftPrefix + params.ncID, OrchestratorContext: context, IPConfiguration: ipConfig, PrimaryInterfaceIdentifier: "11.0.0.7", @@ -572,6 +683,33 @@ func getNonExistNetworkContainerByContext(t *testing.T, name string) error { return nil } +func getNetworkContainerByContextExpectedError(t *testing.T, name string) error { + var body bytes.Buffer + var resp cns.GetNetworkContainerResponse + + podInfo := cns.KubernetesPodInfo{PodName: "testpod", PodNamespace: "testpodnamespace"} + podInfoBytes, err := json.Marshal(podInfo) + getReq := &cns.GetNetworkContainerRequest{OrchestratorContext: podInfoBytes} + + json.NewEncoder(&body).Encode(getReq) + req, err := http.NewRequest(http.MethodPost, cns.GetNetworkContainerByOrchestratorContext, &body) + if err != nil { + t.Fatal(err) + } + + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + err = decodeResponse(w, &resp) + if err != nil || resp.Response.ReturnCode == 0 { + t.Errorf("GetNetworkContainerByContext failed with response %+v Err:%+v", resp, err) + t.Fatal(err) + } + + fmt.Printf("**getNetworkContainerByContextExpectedError succeded with response %+v, raw:%+v\n", resp, w.Body) + return nil +} + func getNetworkContainerStatus(t *testing.T, name string) error { var body bytes.Buffer var resp cns.GetNetworkContainerStatusResponse From dfa16f20fd571448187256367d38705d3e22d900 Mon Sep 17 00:00:00 2001 From: Ashvin Deodhar Date: Tue, 6 Oct 2020 12:09:33 -0700 Subject: [PATCH 7/9] Complete test cases --- cns/restserver/api.go | 3 + cns/restserver/api_test.go | 112 ++++++++++++++++++++++++++++++------- 2 files changed, 94 insertions(+), 21 deletions(-) diff --git a/cns/restserver/api.go b/cns/restserver/api.go index e38a10cdb7..56432f136a 100644 --- a/cns/restserver/api.go +++ b/cns/restserver/api.go @@ -1312,6 +1312,9 @@ func (service *HTTPRestService) unpublishNetworkContainer(w http.ResponseWriter, unpublishResponse.Body.Close() } } + + // Remove the NC version URL entry added during publish + ncVersionURLs.Delete(cns.SwiftPrefix + req.NetworkContainerID) default: returnMessage = "UnpublishNetworkContainer API expects a POST" returnCode = UnsupportedVerb diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 70801b9233..e83cc1fcb7 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -52,11 +52,10 @@ type xmlDocument struct { } var ( - service cns.HTTPService - svc *HTTPRestService - mux *http.ServeMux - hostQueryForProgrammedVersionResponse = `{"httpStatusCode":"200","networkContainerId":"ethWebApp","version":"0"}` - hostQueryResponse = xmlDocument{ + service cns.HTTPService + svc *HTTPRestService + mux *http.ServeMux + hostQueryResponse = xmlDocument{ XMLName: xml.Name{Local: "Interfaces"}, Interface: []Interface{Interface{ XMLName: xml.Name{Local: "Interface"}, @@ -85,6 +84,7 @@ type createOrUpdateNetworkContainerParams struct { ncIP string ncType string ncVersion string + vnetID string } func getInterfaceInfo(w http.ResponseWriter, r *http.Request) { @@ -95,10 +95,25 @@ func getInterfaceInfo(w http.ResponseWriter, r *http.Request) { func nmagentHandler(w http.ResponseWriter, r *http.Request) { w.Header().Set(acncommon.ContentType, acncommon.JsonContent) - w.WriteHeader(http.StatusOK) - if strings.Contains(r.RequestURI, "networkContainers") { - w.Write([]byte(hostQueryForProgrammedVersionResponse)) + if strings.Contains(r.RequestURI, "nc-nma-success") { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"httpStatusCode":"200","networkContainerId":"nc-nma-success","version":"0"}`)) + } + + if strings.Contains(r.RequestURI, "nc-nma-fail-version-mismatch") { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"httpStatusCode":"200","networkContainerId":"nc-nma-fail-version-mismatch","version":"0"}`)) + } + + if strings.Contains(r.RequestURI, "nc-nma-fail-500") { + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte(`{"httpStatusCode":"200","networkContainerId":"nc-nma-fail-500","version":"0"}`)) + } + + if strings.Contains(r.RequestURI, "nc-nma-fail-unavailable") { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"httpStatusCode":"401","networkContainerId":"nc-nma-fail-unavailable","version":"0"}`)) } } @@ -398,54 +413,109 @@ func TestGetNetworkContainerVersionStatus(t *testing.T) { setOrchestratorType(t, cns.Kubernetes) params := createOrUpdateNetworkContainerParams{ - ncID: "ethWebApp", + ncID: "nc-nma-success", ncIP: "11.0.0.5", ncType: cns.AzureContainerInstance, ncVersion: "0", + vnetID: "vnet1", } - if err := createOrUpdateNetworkContainerWithParams(t, params); err != nil { - t.Errorf("createOrUpdateNetworkContainerWithParams failed Err:%+v", err) + createNC(t, params) + + if err := getNetworkContainerByContext(t, "nc-nma-success"); err != nil { + t.Errorf("TestGetNetworkContainerByOrchestratorContext failed Err:%+v", err) t.Fatal(err) } - publishNCViaCNS(t, "vnet1", "ethWebApp") - - if err := getNetworkContainerByContext(t, "ethWebApp"); err != nil { + // Get NC goal state again to test the path where the NMA API doesn't need to be executed but + // instead use the cached state ( in json ) of version status + if err := getNetworkContainerByContext(t, "nc-nma-success"); err != nil { t.Errorf("TestGetNetworkContainerByOrchestratorContext failed Err:%+v", err) t.Fatal(err) } - if err := deleteNetworkAdapterWithName(t, "ethWebApp"); err != nil { + if err := deleteNetworkAdapterWithName(t, "nc-nma-success"); err != nil { t.Errorf("Deleting interface failed Err:%+v", err) t.Fatal(err) } + // Testing the path where the NC version with CNS is higher than the one with NMAgent. + // This indicates that the NMAgent is yet to program the NC version. params = createOrUpdateNetworkContainerParams{ - ncID: "ethWebAppUpdated", + ncID: "nc-nma-fail-version-mismatch", ncIP: "11.0.0.5", ncType: cns.AzureContainerInstance, ncVersion: "1", + vnetID: "vnet1", } - if err := createOrUpdateNetworkContainerWithParams(t, params); err != nil { - t.Errorf("createOrUpdateNetworkContainerWithParams failed Err:%+v", err) + createNC(t, params) + + if err := getNetworkContainerByContextExpectedError(t, "nc-nma-fail-version-mismatch"); err != nil { + t.Errorf("TestGetNetworkContainerVersionStatus failed") t.Fatal(err) } - publishNCViaCNS(t, "vnet1", "ethWebAppUpdated") + if err := deleteNetworkAdapterWithName(t, "nc-nma-fail-version-mismatch"); err != nil { + t.Errorf("Deleting interface failed Err:%+v", err) + t.Fatal(err) + } - if err := getNetworkContainerByContextExpectedError(t, "ethWebAppUpdated"); err != nil { + // Testing the path where NMAgent response status code is not 200. + // 2. NMAgent response status code is 200 but embedded response is 401 + params = createOrUpdateNetworkContainerParams{ + ncID: "nc-nma-fail-500", + ncIP: "11.0.0.5", + ncType: cns.AzureContainerInstance, + ncVersion: "0", + vnetID: "vnet1", + } + + createNC(t, params) + + if err := getNetworkContainerByContext(t, "nc-nma-fail-500"); err != nil { + t.Errorf("TestGetNetworkContainerVersionStatus failed") + t.Fatal(err) + } + + if err := deleteNetworkAdapterWithName(t, "nc-nma-fail-500"); err != nil { + t.Errorf("Deleting interface failed Err:%+v", err) + t.Fatal(err) + } + + // Testing the path where NMAgent response status code is 200 but embedded response is 401 + params = createOrUpdateNetworkContainerParams{ + ncID: "nc-nma-fail-unavailable", + ncIP: "11.0.0.5", + ncType: cns.AzureContainerInstance, + ncVersion: "0", + vnetID: "vnet1", + } + + createNC(t, params) + + if err := getNetworkContainerByContextExpectedError(t, "nc-nma-fail-unavailable"); err != nil { t.Errorf("TestGetNetworkContainerVersionStatus failed") t.Fatal(err) } - if err := deleteNetworkAdapterWithName(t, "ethWebAppUpdated"); err != nil { + if err := deleteNetworkAdapterWithName(t, "nc-nma-fail-unavailable"); err != nil { t.Errorf("Deleting interface failed Err:%+v", err) t.Fatal(err) } } +func createNC( + t *testing.T, + params createOrUpdateNetworkContainerParams) { + if err := createOrUpdateNetworkContainerWithParams(t, params); err != nil { + t.Errorf("createOrUpdateNetworkContainerWithParams failed Err:%+v", err) + t.Fatal(err) + } + + publishNCViaCNS(t, params.vnetID, params.ncID) +} + func TestPublishNCViaCNS(t *testing.T) { fmt.Println("Test: publishNetworkContainer") publishNCViaCNS(t, "vnet1", "ethWebApp") From f532c11510e7f7a0fa0f67a58a14ffcafb76029e Mon Sep 17 00:00:00 2001 From: Ashvin Deodhar Date: Mon, 2 Nov 2020 14:19:29 -0800 Subject: [PATCH 8/9] Fix the test --- cns/restserver/api_test.go | 209 +++++++++++++++++++------------------ 1 file changed, 105 insertions(+), 104 deletions(-) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index b51797aa78..933c84280f 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -80,11 +80,13 @@ const ( ) type createOrUpdateNetworkContainerParams struct { - ncID string - ncIP string - ncType string - ncVersion string - vnetID string + ncID string + ncIP string + ncType string + ncVersion string + vnetID string + podName string + podNamespace string } func getInterfaceInfo(w http.ResponseWriter, r *http.Request) { @@ -193,10 +195,12 @@ func TestCreateNetworkContainer(t *testing.T) { fmt.Println("TestCreateNetworkContainer: JobObject") params := createOrUpdateNetworkContainerParams{ - ncID: "testJobObject", - ncIP: "10.1.0.5", - ncType: "JobObject", - ncVersion: "0", + ncID: "testJobObject", + ncIP: "10.1.0.5", + ncType: "JobObject", + ncVersion: "0", + podName: "testpod", + podNamespace: "testpodnamespace", } err := createOrUpdateNetworkContainerWithParams(t, params) @@ -207,7 +211,7 @@ func TestCreateNetworkContainer(t *testing.T) { } fmt.Println("Deleting the saved goal state for network container of type JobObject") - err = deleteNetworkAdapterWithName(t, "testJobObject") + err = deleteNetworkContainerWithParams(t, params) if err != nil { t.Errorf("Failed to delete the saved goal state due to error: %+v", err) t.Fatal(err) @@ -216,10 +220,12 @@ func TestCreateNetworkContainer(t *testing.T) { // Test create network container of type WebApps fmt.Println("TestCreateNetworkContainer: WebApps") params = createOrUpdateNetworkContainerParams{ - ncID: "ethWebApp", - ncIP: "192.0.0.5", - ncType: "WebApps", - ncVersion: "0", + ncID: "ethWebApp", + ncIP: "192.0.0.5", + ncType: "WebApps", + ncVersion: "0", + podName: "testpod", + podNamespace: "testpodnamespace", } err = createOrUpdateNetworkContainerWithParams(t, params) @@ -229,10 +235,12 @@ func TestCreateNetworkContainer(t *testing.T) { } params = createOrUpdateNetworkContainerParams{ - ncID: "ethWebApp", - ncIP: "192.0.0.6", - ncType: "WebApps", - ncVersion: "0", + ncID: "ethWebApp", + ncIP: "192.0.0.6", + ncType: "WebApps", + ncVersion: "0", + podName: "testpod", + podNamespace: "testpodnamespace", } err = createOrUpdateNetworkContainerWithParams(t, params) @@ -243,7 +251,7 @@ func TestCreateNetworkContainer(t *testing.T) { fmt.Println("Now calling DeleteNetworkContainer") - err = deleteNetworkAdapterWithName(t, "ethWebApp") + err = deleteNetworkContainerWithParams(t, params) if err != nil { t.Errorf("Deleting interface failed Err:%+v", err) t.Fatal(err) @@ -251,10 +259,12 @@ func TestCreateNetworkContainer(t *testing.T) { // Test create network container of type COW params = createOrUpdateNetworkContainerParams{ - ncID: "testCOWContainer", - ncIP: "10.0.0.5", - ncType: "COW", - ncVersion: "0", + ncID: "testCOWContainer", + ncIP: "10.0.0.5", + ncType: "COW", + ncVersion: "0", + podName: "testpod", + podNamespace: "testpodnamespace", } err = createOrUpdateNetworkContainerWithParams(t, params) @@ -265,7 +275,7 @@ func TestCreateNetworkContainer(t *testing.T) { } fmt.Println("Deleting the saved goal state for network container of type COW") - err = deleteNetworkAdapterWithName(t, "testCOWContainer") + err = deleteNetworkContainerWithParams(t, params) if err != nil { t.Errorf("Failed to delete the saved goal state due to error: %+v", err) t.Fatal(err) @@ -281,10 +291,12 @@ func TestGetNetworkContainerByOrchestratorContext(t *testing.T) { setOrchestratorType(t, cns.Kubernetes) params := createOrUpdateNetworkContainerParams{ - ncID: "ethWebApp", - ncIP: "11.0.0.5", - ncType: cns.AzureContainerInstance, - ncVersion: "0", + ncID: "ethWebApp", + ncIP: "11.0.0.5", + ncType: cns.AzureContainerInstance, + ncVersion: "0", + podName: "testpod", + podNamespace: "testpodnamespace", } err := createOrUpdateNetworkContainerWithParams(t, params) @@ -294,7 +306,7 @@ func TestGetNetworkContainerByOrchestratorContext(t *testing.T) { } fmt.Println("Now calling getNetworkContainerStatus") - err = getNetworkContainerByContext(t, "ethWebApp") + err = getNetworkContainerByContext(t, params) if err != nil { t.Errorf("TestGetNetworkContainerByOrchestratorContext failed Err:%+v", err) t.Fatal(err) @@ -302,48 +314,19 @@ func TestGetNetworkContainerByOrchestratorContext(t *testing.T) { fmt.Println("Now calling DeleteNetworkContainer") - err = deleteNetworkAdapterWithName(t, "ethWebApp") + err = deleteNetworkContainerWithParams(t, params) if err != nil { t.Errorf("Deleting interface failed Err:%+v", err) t.Fatal(err) } - err = getNonExistNetworkContainerByContext(t, "ethWebApp") + err = getNonExistNetworkContainerByContext(t, params) if err != nil { t.Errorf("TestGetNetworkContainerByOrchestratorContext failed Err:%+v", err) t.Fatal(err) } } -// func TestGetNetworkContainerStatus(t *testing.T) { -// // requires more than 30 seconds to run -// fmt.Println("Test: TestCreateNetworkContainer") - -// setEnv(t) -// setOrchestratorType(t, cns.Kubernetes) - -// err := createOrUpdateNetworkContainerWithParams(t, "ethWebApp", "11.0.0.5", cns.AzureContainerInstance) -// if err != nil { -// t.Errorf("creatOrUpdateWebAppContainerWithName failed Err:%+v", err) -// t.Fatal(err) -// } - -// fmt.Println("Now calling getNetworkContainerStatus") -// err = getNetworkContainerStatus(t, "ethWebApp") -// if err != nil { -// t.Errorf("getNetworkContainerStatus failed Err:%+v", err) -// t.Fatal(err) -// } - -// fmt.Println("Now calling DeleteNetworkContainer") - -// err = deleteNetworkAdapterWithName(t, "ethWebApp") -// if err != nil { -// t.Errorf("Deleting interface failed Err:%+v", err) -// t.Fatal(err) -// } -// } - func TestGetInterfaceForNetworkContainer(t *testing.T) { // requires more than 30 seconds to run fmt.Println("Test: TestCreateNetworkContainer") @@ -352,10 +335,12 @@ func TestGetInterfaceForNetworkContainer(t *testing.T) { setOrchestratorType(t, cns.Kubernetes) params := createOrUpdateNetworkContainerParams{ - ncID: "ethWebApp", - ncIP: "11.0.0.5", - ncType: "WebApps", - ncVersion: "0", + ncID: "ethWebApp", + ncIP: "11.0.0.5", + ncType: "WebApps", + ncVersion: "0", + podName: "testpod", + podNamespace: "testpodnamespace", } err := createOrUpdateNetworkContainerWithParams(t, params) @@ -373,7 +358,7 @@ func TestGetInterfaceForNetworkContainer(t *testing.T) { fmt.Println("Now calling DeleteNetworkContainer") - err = deleteNetworkAdapterWithName(t, "ethWebApp") + err = deleteNetworkContainerWithParams(t, params) if err != nil { t.Errorf("Deleting interface failed Err:%+v", err) t.Fatal(err) @@ -413,28 +398,30 @@ func TestGetNetworkContainerVersionStatus(t *testing.T) { setOrchestratorType(t, cns.Kubernetes) params := createOrUpdateNetworkContainerParams{ - ncID: "nc-nma-success", - ncIP: "11.0.0.5", - ncType: cns.AzureContainerInstance, - ncVersion: "0", - vnetID: "vnet1", + ncID: "nc-nma-success", + ncIP: "11.0.0.5", + ncType: cns.AzureContainerInstance, + ncVersion: "0", + vnetID: "vnet1", + podName: "testpod", + podNamespace: "testpodnamespace", } createNC(t, params) - if err := getNetworkContainerByContext(t, "nc-nma-success"); err != nil { + if err := getNetworkContainerByContext(t, params); err != nil { t.Errorf("TestGetNetworkContainerByOrchestratorContext failed Err:%+v", err) t.Fatal(err) } // Get NC goal state again to test the path where the NMA API doesn't need to be executed but // instead use the cached state ( in json ) of version status - if err := getNetworkContainerByContext(t, "nc-nma-success"); err != nil { + if err := getNetworkContainerByContext(t, params); err != nil { t.Errorf("TestGetNetworkContainerByOrchestratorContext failed Err:%+v", err) t.Fatal(err) } - if err := deleteNetworkAdapterWithName(t, "nc-nma-success"); err != nil { + if err := deleteNetworkContainerWithParams(t, params); err != nil { t.Errorf("Deleting interface failed Err:%+v", err) t.Fatal(err) } @@ -442,21 +429,23 @@ func TestGetNetworkContainerVersionStatus(t *testing.T) { // Testing the path where the NC version with CNS is higher than the one with NMAgent. // This indicates that the NMAgent is yet to program the NC version. params = createOrUpdateNetworkContainerParams{ - ncID: "nc-nma-fail-version-mismatch", - ncIP: "11.0.0.5", - ncType: cns.AzureContainerInstance, - ncVersion: "1", - vnetID: "vnet1", + ncID: "nc-nma-fail-version-mismatch", + ncIP: "11.0.0.5", + ncType: cns.AzureContainerInstance, + ncVersion: "1", + vnetID: "vnet1", + podName: "testpod", + podNamespace: "testpodnamespace", } createNC(t, params) - if err := getNetworkContainerByContextExpectedError(t, "nc-nma-fail-version-mismatch"); err != nil { + if err := getNetworkContainerByContextExpectedError(t, params); err != nil { t.Errorf("TestGetNetworkContainerVersionStatus failed") t.Fatal(err) } - if err := deleteNetworkAdapterWithName(t, "nc-nma-fail-version-mismatch"); err != nil { + if err := deleteNetworkContainerWithParams(t, params); err != nil { t.Errorf("Deleting interface failed Err:%+v", err) t.Fatal(err) } @@ -464,42 +453,46 @@ func TestGetNetworkContainerVersionStatus(t *testing.T) { // Testing the path where NMAgent response status code is not 200. // 2. NMAgent response status code is 200 but embedded response is 401 params = createOrUpdateNetworkContainerParams{ - ncID: "nc-nma-fail-500", - ncIP: "11.0.0.5", - ncType: cns.AzureContainerInstance, - ncVersion: "0", - vnetID: "vnet1", + ncID: "nc-nma-fail-500", + ncIP: "11.0.0.5", + ncType: cns.AzureContainerInstance, + ncVersion: "0", + vnetID: "vnet1", + podName: "testpod", + podNamespace: "testpodnamespace", } createNC(t, params) - if err := getNetworkContainerByContext(t, "nc-nma-fail-500"); err != nil { + if err := getNetworkContainerByContext(t, params); err != nil { t.Errorf("TestGetNetworkContainerVersionStatus failed") t.Fatal(err) } - if err := deleteNetworkAdapterWithName(t, "nc-nma-fail-500"); err != nil { + if err := deleteNetworkContainerWithParams(t, params); err != nil { t.Errorf("Deleting interface failed Err:%+v", err) t.Fatal(err) } // Testing the path where NMAgent response status code is 200 but embedded response is 401 params = createOrUpdateNetworkContainerParams{ - ncID: "nc-nma-fail-unavailable", - ncIP: "11.0.0.5", - ncType: cns.AzureContainerInstance, - ncVersion: "0", - vnetID: "vnet1", + ncID: "nc-nma-fail-unavailable", + ncIP: "11.0.0.5", + ncType: cns.AzureContainerInstance, + ncVersion: "0", + vnetID: "vnet1", + podName: "testpod", + podNamespace: "testpodnamespace", } createNC(t, params) - if err := getNetworkContainerByContextExpectedError(t, "nc-nma-fail-unavailable"); err != nil { + if err := getNetworkContainerByContextExpectedError(t, params); err != nil { t.Errorf("TestGetNetworkContainerVersionStatus failed") t.Fatal(err) } - if err := deleteNetworkAdapterWithName(t, "nc-nma-fail-unavailable"); err != nil { + if err := deleteNetworkContainerWithParams(t, params); err != nil { t.Errorf("Deleting interface failed Err:%+v", err) t.Fatal(err) } @@ -672,12 +665,14 @@ func createOrUpdateNetworkContainerWithParams(t *testing.T, params createOrUpdat return nil } -func deleteNetworkAdapterWithName(t *testing.T, name string) error { - var body bytes.Buffer - var resp cns.DeleteNetworkContainerResponse +func deleteNetworkContainerWithParams(t *testing.T, params createOrUpdateNetworkContainerParams) error { + var ( + body bytes.Buffer + resp cns.DeleteNetworkContainerResponse + ) deleteInfo := &cns.DeleteNetworkContainerRequest{ - NetworkContainerid: name, + NetworkContainerid: cns.SwiftPrefix + params.ncID, } json.NewEncoder(&body).Encode(deleteInfo) @@ -699,11 +694,13 @@ func deleteNetworkAdapterWithName(t *testing.T, name string) error { return nil } -func getNetworkContainerByContext(t *testing.T, name string) error { +func getNetworkContainerByContext(t *testing.T, params createOrUpdateNetworkContainerParams) error { var body bytes.Buffer var resp cns.GetNetworkContainerResponse + podInfo := cns.KubernetesPodInfo{ + PodName: params.podName, + PodNamespace: params.podNamespace} - podInfo := cns.KubernetesPodInfo{PodName: "testpod", PodNamespace: "testpodnamespace"} podInfoBytes, err := json.Marshal(podInfo) getReq := &cns.GetNetworkContainerRequest{OrchestratorContext: podInfoBytes} @@ -726,11 +723,13 @@ func getNetworkContainerByContext(t *testing.T, name string) error { return nil } -func getNonExistNetworkContainerByContext(t *testing.T, name string) error { +func getNonExistNetworkContainerByContext(t *testing.T, params createOrUpdateNetworkContainerParams) error { var body bytes.Buffer var resp cns.GetNetworkContainerResponse + podInfo := cns.KubernetesPodInfo{ + PodName: params.podName, + PodNamespace: params.podNamespace} - podInfo := cns.KubernetesPodInfo{PodName: "testpod", PodNamespace: "testpodnamespace"} podInfoBytes, err := json.Marshal(podInfo) getReq := &cns.GetNetworkContainerRequest{OrchestratorContext: podInfoBytes} @@ -753,11 +752,13 @@ func getNonExistNetworkContainerByContext(t *testing.T, name string) error { return nil } -func getNetworkContainerByContextExpectedError(t *testing.T, name string) error { +func getNetworkContainerByContextExpectedError(t *testing.T, params createOrUpdateNetworkContainerParams) error { var body bytes.Buffer var resp cns.GetNetworkContainerResponse + podInfo := cns.KubernetesPodInfo{ + PodName: params.podName, + PodNamespace: params.podNamespace} - podInfo := cns.KubernetesPodInfo{PodName: "testpod", PodNamespace: "testpodnamespace"} podInfoBytes, err := json.Marshal(podInfo) getReq := &cns.GetNetworkContainerRequest{OrchestratorContext: podInfoBytes} From d6cc9779777736bffa87edd2d9ceb378eec71834 Mon Sep 17 00:00:00 2001 From: Ashvin Deodhar Date: Mon, 2 Nov 2020 14:41:43 -0800 Subject: [PATCH 9/9] Fix test --- cns/restserver/api_test.go | 35 ++++------------------------------- 1 file changed, 4 insertions(+), 31 deletions(-) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 933c84280f..9d2199fbac 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -305,7 +305,7 @@ func TestGetNetworkContainerByOrchestratorContext(t *testing.T) { t.Fatal(err) } - fmt.Println("Now calling getNetworkContainerStatus") + fmt.Println("Now calling getNetworkContainerByContext") err = getNetworkContainerByContext(t, params) if err != nil { t.Errorf("TestGetNetworkContainerByOrchestratorContext failed Err:%+v", err) @@ -350,7 +350,7 @@ func TestGetInterfaceForNetworkContainer(t *testing.T) { } fmt.Println("Now calling getInterfaceForContainer") - err = getInterfaceForContainer(t, "ethWebApp") + err = getInterfaceForContainer(t, params) if err != nil { t.Errorf("getInterfaceForContainer failed Err:%+v", err) t.Fatal(err) @@ -781,39 +781,12 @@ func getNetworkContainerByContextExpectedError(t *testing.T, params createOrUpda return nil } -func getNetworkContainerStatus(t *testing.T, name string) error { - var body bytes.Buffer - var resp cns.GetNetworkContainerStatusResponse - - getReq := &cns.GetNetworkContainerStatusRequest{ - NetworkContainerid: name, - } - - json.NewEncoder(&body).Encode(getReq) - req, err := http.NewRequest(http.MethodPost, cns.GetNetworkContainerStatus, &body) - if err != nil { - t.Fatal(err) - } - - w := httptest.NewRecorder() - mux.ServeHTTP(w, req) - - err = decodeResponse(w, &resp) - if err != nil || resp.Response.ReturnCode != 0 { - t.Errorf("GetNetworkContainerStatus failed with response %+v Err:%+v", resp, err) - t.Fatal(err) - } - - fmt.Printf("**GetNetworkContainerStatus succeded with response %+v, raw:%+v\n", resp, w.Body) - return nil -} - -func getInterfaceForContainer(t *testing.T, name string) error { +func getInterfaceForContainer(t *testing.T, params createOrUpdateNetworkContainerParams) error { var body bytes.Buffer var resp cns.GetInterfaceForContainerResponse getReq := &cns.GetInterfaceForContainerRequest{ - NetworkContainerID: name, + NetworkContainerID: cns.SwiftPrefix + params.ncID, } json.NewEncoder(&body).Encode(getReq)