From a9c49159ced1c2ba93b38028d318fabf0bdd1d1a Mon Sep 17 00:00:00 2001 From: Shufang Date: Mon, 12 Oct 2020 20:53:44 -0700 Subject: [PATCH 01/15] Add pending program status for IPs in CNS. Add logic structure of how to update program status. --- cns/NetworkContainerContract.go | 1 + cns/api.go | 1 + cns/fakes/cnsfake.go | 4 ++ cns/ipampoolmonitor/ipampoolmonitor.go | 4 +- .../kubecontroller/crdtranslator.go | 2 +- cns/restserver/ipam.go | 9 +++ cns/restserver/util.go | 60 +++++++++++++------ 7 files changed, 61 insertions(+), 20 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 0f97dbb5ad..3742b28827 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -61,6 +61,7 @@ const ( Available = "Available" Allocated = "Allocated" PendingRelease = "PendingRelease" + PendingProgram = "PendingProgram" ) // ChannelMode :- CNS channel modes diff --git a/cns/api.go b/cns/api.go index 0c06500e28..113fcc7b3d 100644 --- a/cns/api.go +++ b/cns/api.go @@ -37,6 +37,7 @@ type HTTPService interface { SendNCSnapShotPeriodically(int, chan bool) SetNodeOrchestrator(*SetOrchestratorTypeRequest) SyncNodeStatus(string, string, string, json.RawMessage) (int, string) + GetPendingProgramIPConfigs() []IPConfigurationStatus GetAvailableIPConfigs() []IPConfigurationStatus GetAllocatedIPConfigs() []IPConfigurationStatus GetPendingReleaseIPConfigs() []IPConfigurationStatus diff --git a/cns/fakes/cnsfake.go b/cns/fakes/cnsfake.go index 80afdad04d..0b02a08a25 100644 --- a/cns/fakes/cnsfake.go +++ b/cns/fakes/cnsfake.go @@ -58,6 +58,7 @@ func (stack *StringStack) Pop() (string, error) { } type IPStateManager struct { + PendingProgramIPConfigState map[string]cns.IPConfigurationStatus AvailableIPConfigState map[string]cns.IPConfigurationStatus AllocatedIPConfigState map[string]cns.IPConfigurationStatus PendingReleaseIPConfigState map[string]cns.IPConfigurationStatus @@ -67,6 +68,7 @@ type IPStateManager struct { func NewIPStateManager() IPStateManager { return IPStateManager{ + PendingProgramIPConfigState: make(map[string]cns.IPConfigurationStatus), AvailableIPConfigState: make(map[string]cns.IPConfigurationStatus), AllocatedIPConfigState: make(map[string]cns.IPConfigurationStatus), PendingReleaseIPConfigState: make(map[string]cns.IPConfigurationStatus), @@ -81,6 +83,8 @@ func (ipm *IPStateManager) AddIPConfigs(ipconfigs []cns.IPConfigurationStatus) { for i := 0; i < len(ipconfigs); i++ { switch { + case ipconfigs[i].State == cns.PendingProgram: + ipm.PendingProgramIPConfigState[ipconfigs[i].ID] = ipconfigs[i] case ipconfigs[i].State == cns.Available: ipm.AvailableIPConfigState[ipconfigs[i].ID] = ipconfigs[i] ipm.AvailableIPIDStack.Push(ipconfigs[i].ID) diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampoolmonitor/ipampoolmonitor.go index 716ea3ce84..822b941c97 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampoolmonitor/ipampoolmonitor.go @@ -64,12 +64,14 @@ func (pm *CNSIPAMPoolMonitor) Start(ctx context.Context, poolMonitorRefreshMilli func (pm *CNSIPAMPoolMonitor) Reconcile() error { cnsPodIPConfigCount := len(pm.cns.GetPodIPConfigState()) + pendingProgramCount := len(pm.cns.GetPendingProgramIPConfigs()) allocatedPodIPCount := len(pm.cns.GetAllocatedIPConfigs()) pendingReleaseIPCount := len(pm.cns.GetPendingReleaseIPConfigs()) availableIPConfigCount := len(pm.cns.GetAvailableIPConfigs()) // TODO: add pending allocation count to real cns freeIPConfigCount := pm.cachedNNC.Spec.RequestedIPCount - int64(allocatedPodIPCount) - logger.Printf("[ipam-pool-monitor] Checking pool for resize, Pool Size: %v, Goal Size: %v, BatchSize: %v, MinFree: %v, MaxFree:%v, Allocated: %v, Available: %v, Pending Release: %v, Free: %v", cnsPodIPConfigCount, pm.cachedNNC.Spec.RequestedIPCount, pm.scalarUnits.BatchSize, pm.MinimumFreeIps, pm.MaximumFreeIps, allocatedPodIPCount, availableIPConfigCount, pendingReleaseIPCount, freeIPConfigCount) + logger.Printf("[ipam-pool-monitor] Checking pool for resize, Pool Size: %v, Goal Size: %v, BatchSize: %v, MinFree: %v, MaxFree:%v, Allocated: %v, Available: %v, Pending Release: %v, Free: %v, Pending Program: %v", + cnsPodIPConfigCount, pm.cachedNNC.Spec.RequestedIPCount, pm.scalarUnits.BatchSize, pm.MinimumFreeIps, pm.MaximumFreeIps, allocatedPodIPCount, availableIPConfigCount, pendingReleaseIPCount, freeIPConfigCount, pendingProgramCount) switch { // pod count is increasing diff --git a/cns/requestcontroller/kubecontroller/crdtranslator.go b/cns/requestcontroller/kubecontroller/crdtranslator.go index aa8c9e89cd..33aa0cc5b3 100644 --- a/cns/requestcontroller/kubecontroller/crdtranslator.go +++ b/cns/requestcontroller/kubecontroller/crdtranslator.go @@ -36,7 +36,7 @@ func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetw ncRequest.SecondaryIPConfigs = make(map[string]cns.SecondaryIPConfig) ncRequest.NetworkContainerid = nc.ID ncRequest.NetworkContainerType = cns.Docker - ncRequest.Version = nc.Version + ncRequest.NCVersion = nc.Version if ip = net.ParseIP(nc.PrimaryIP); ip == nil { return ncRequest, fmt.Errorf("Invalid PrimaryIP %s:", nc.PrimaryIP) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 3b6baea8ec..adadf648d6 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -118,6 +118,15 @@ func (service *HTTPRestService) GetPodIPConfigState() map[string]cns.IPConfigura return service.PodIPConfigState } +// GetPendingProgramIPConfigs returns list of IPs which are in pending program status +func (service *HTTPRestService) GetPendingProgramIPConfigs() []cns.IPConfigurationStatus { + service.RLock() + defer service.RUnlock() + return filterIPConfigMap(service.PodIPConfigState, func(ipconfig cns.IPConfigurationStatus) bool { + return ipconfig.State == cns.PendingProgram + }) +} + func (service *HTTPRestService) GetAllocatedIPConfigs() []cns.IPConfigurationStatus { service.RLock() defer service.RUnlock() diff --git a/cns/restserver/util.go b/cns/restserver/util.go index b493bd160a..b60abad5ed 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -7,6 +7,7 @@ import ( "encoding/json" "fmt" "io/ioutil" + "math" "net/http" "strconv" "time" @@ -236,7 +237,7 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN } // Add the newIpConfigs, ignore if ip state is already in the map - service.addIPConfigStateUntransacted(req.NetworkContainerid, newIPConfigs) + service.addIPConfigStateUntransacted(req.NCVersion, req.NetworkContainerid, newIPConfigs) return 0, "" } @@ -244,29 +245,52 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN // addIPConfigStateUntransacted adds the IPConfigs to the PodIpConfigState map with Available state // If the IP is already added then it will be an idempotent call. Also note, caller will // acquire/release the service lock. -func (service *HTTPRestService) addIPConfigStateUntransacted(ncId string, ipconfigs map[string]cns.SecondaryIPConfig) { - // add ipconfigs to state - for ipId, ipconfig := range ipconfigs { - // if this IPConfig already exists in the map, then ignore as this is an idempotent state - if _, exists := service.PodIPConfigState[ipId]; exists { - continue - } +func (service *HTTPRestService) addIPConfigStateUntransacted(ncVersion, ncId string, ipconfigs map[string]cns.SecondaryIPConfig) { + + existingNCStatus, _ := service.state.ContainerStatus[ncId] + existingNCVersion, _ := strconv.Atoi(existingNCStatus.CreateNetworkContainerRequest.NCVersion) + nmAgentNCVersion := getNCVersionFromNMAgent(ncId) + newNCVersion, _ := strconv.Atoi(ncVersion) + + if nmAgentNCVersion >= newNCVersion { // add ipconfigs to state + for ipId, ipconfig := range ipconfigs { + // if this IPConfig already exists in the map, then ignore as this is an idempotent state + if _, exists := service.PodIPConfigState[ipId]; exists { + continue + } - // add the new State - ipconfigStatus := cns.IPConfigurationStatus{ - NCID: ncId, - ID: ipId, - IPAddress: ipconfig.IPAddress, - State: cns.Available, - OrchestratorContext: nil, - } + // add the new State + ipconfigStatus := cns.IPConfigurationStatus{ + NCID: ncId, + ID: ipId, + IPAddress: ipconfig.IPAddress, + State: cns.Available, + OrchestratorContext: nil, + } + + service.PodIPConfigState[ipId] = ipconfigStatus - service.PodIPConfigState[ipId] = ipconfigStatus + // Todo Update batch API and maintain the count - // Todo Update batch API and maintain the count + existingNCStatus.CreateNetworkContainerRequest.NCVersion = strconv.Itoa(newNCVersion) + } + } else { + if nmAgentNCVersion >= existingNCVersion { + // todo, update all pending program IP to available. + // Unconmment the following command when getNCVersionFromNMAgent function finished. + // existingNCStatus.CreateNetworkContainerRequest.NCVersion = strconv.Itoa(nmAgentNCVersion) + } else { + // todo, update IPs in ipconfigs which is not in the map to pending program. + } } } +// To do, complete this logic according to getNetworkContainerStatus +func getNCVersionFromNMAgent(ncid string) int { + // return a largest int to not break current. + return math.MaxInt64 +} + // Todo: call this when request is received func validateIPSubnet(ipSubnet cns.IPSubnet) error { if ipSubnet.IPAddress == "" { From 29ef4344a33eb9856187423b188be9b144054d65 Mon Sep 17 00:00:00 2001 From: Shufang Date: Wed, 14 Oct 2020 15:58:01 -0700 Subject: [PATCH 02/15] Add missing NCVersion CreateNetworkContainerRequest in commit. --- cns/NetworkContainerContract.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 3742b28827..811ef2d0e0 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -74,6 +74,7 @@ const ( // CreateNetworkContainerRequest specifies request to create a network container or network isolation boundary. type CreateNetworkContainerRequest struct { Version string + NCVersion string NetworkContainerType string NetworkContainerid string // Mandatory input. PrimaryInterfaceIdentifier string // Primary CA. From 0f24d45d3e43a485228c47fe1c4afeb687a79de2 Mon Sep 17 00:00:00 2001 From: Shufang Date: Wed, 14 Oct 2020 21:44:42 -0700 Subject: [PATCH 03/15] Add missing fake GetPendingProgramIPConfigs to unblock ipam pool unit test. --- cns/fakes/cnsfake.go | 9 +++++++++ cns/ipampoolmonitor/ipampoolmonitor.go | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/cns/fakes/cnsfake.go b/cns/fakes/cnsfake.go index 0b02a08a25..06d5cc2c1f 100644 --- a/cns/fakes/cnsfake.go +++ b/cns/fakes/cnsfake.go @@ -225,6 +225,15 @@ func (fake *HTTPServiceFake) SyncNodeStatus(string, string, string, json.RawMess return 0, "" } +func (fake *HTTPServiceFake) GetPendingProgramIPConfigs() []cns.IPConfigurationStatus { + ipconfigs := []cns.IPConfigurationStatus{} + for _, ipconfig := range fake.IPStateManager.PendingProgramIPConfigState { + ipconfigs = append(ipconfigs, ipconfig) + } + + return ipconfigs +} + func (fake *HTTPServiceFake) GetAvailableIPConfigs() []cns.IPConfigurationStatus { ipconfigs := []cns.IPConfigurationStatus{} for _, ipconfig := range fake.IPStateManager.AvailableIPConfigState { diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampoolmonitor/ipampoolmonitor.go index 822b941c97..e8e03b558f 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampoolmonitor/ipampoolmonitor.go @@ -64,7 +64,7 @@ func (pm *CNSIPAMPoolMonitor) Start(ctx context.Context, poolMonitorRefreshMilli func (pm *CNSIPAMPoolMonitor) Reconcile() error { cnsPodIPConfigCount := len(pm.cns.GetPodIPConfigState()) - pendingProgramCount := len(pm.cns.GetPendingProgramIPConfigs()) + pendingProgramCount := len(pm.cns.GetPendingProgramIPConfigs()) // TODO: add pending program count to real cns allocatedPodIPCount := len(pm.cns.GetAllocatedIPConfigs()) pendingReleaseIPCount := len(pm.cns.GetPendingReleaseIPConfigs()) availableIPConfigCount := len(pm.cns.GetAvailableIPConfigs()) // TODO: add pending allocation count to real cns From c48b18367297d8088329e06d97fa65a525fb5ef8 Mon Sep 17 00:00:00 2001 From: Shufang Date: Thu, 15 Oct 2020 18:33:10 -0700 Subject: [PATCH 04/15] Address feedbacks. --- cns/NetworkContainerContract.go | 9 +++---- cns/fakes/cnsfake.go | 2 +- .../kubecontroller/crdtranslator.go | 2 +- cns/restserver/ipam.go | 2 +- cns/restserver/util.go | 25 ++++++++----------- 5 files changed, 17 insertions(+), 23 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 811ef2d0e0..31c06fed2f 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -58,10 +58,10 @@ const ( // IPConfig States for CNS IPAM const ( - Available = "Available" - Allocated = "Allocated" - PendingRelease = "PendingRelease" - PendingProgram = "PendingProgram" + Available = "Available" + Allocated = "Allocated" + PendingRelease = "PendingRelease" + PendingProgramming = "PendingProgramming" ) // ChannelMode :- CNS channel modes @@ -74,7 +74,6 @@ const ( // CreateNetworkContainerRequest specifies request to create a network container or network isolation boundary. type CreateNetworkContainerRequest struct { Version string - NCVersion string NetworkContainerType string NetworkContainerid string // Mandatory input. PrimaryInterfaceIdentifier string // Primary CA. diff --git a/cns/fakes/cnsfake.go b/cns/fakes/cnsfake.go index 06d5cc2c1f..d87a47748f 100644 --- a/cns/fakes/cnsfake.go +++ b/cns/fakes/cnsfake.go @@ -83,7 +83,7 @@ func (ipm *IPStateManager) AddIPConfigs(ipconfigs []cns.IPConfigurationStatus) { for i := 0; i < len(ipconfigs); i++ { switch { - case ipconfigs[i].State == cns.PendingProgram: + case ipconfigs[i].State == cns.PendingProgramming: ipm.PendingProgramIPConfigState[ipconfigs[i].ID] = ipconfigs[i] case ipconfigs[i].State == cns.Available: ipm.AvailableIPConfigState[ipconfigs[i].ID] = ipconfigs[i] diff --git a/cns/requestcontroller/kubecontroller/crdtranslator.go b/cns/requestcontroller/kubecontroller/crdtranslator.go index 33aa0cc5b3..aa8c9e89cd 100644 --- a/cns/requestcontroller/kubecontroller/crdtranslator.go +++ b/cns/requestcontroller/kubecontroller/crdtranslator.go @@ -36,7 +36,7 @@ func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetw ncRequest.SecondaryIPConfigs = make(map[string]cns.SecondaryIPConfig) ncRequest.NetworkContainerid = nc.ID ncRequest.NetworkContainerType = cns.Docker - ncRequest.NCVersion = nc.Version + ncRequest.Version = nc.Version if ip = net.ParseIP(nc.PrimaryIP); ip == nil { return ncRequest, fmt.Errorf("Invalid PrimaryIP %s:", nc.PrimaryIP) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index adadf648d6..1549e19a75 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -123,7 +123,7 @@ func (service *HTTPRestService) GetPendingProgramIPConfigs() []cns.IPConfigurati service.RLock() defer service.RUnlock() return filterIPConfigMap(service.PodIPConfigState, func(ipconfig cns.IPConfigurationStatus) bool { - return ipconfig.State == cns.PendingProgram + return ipconfig.State == cns.PendingProgramming }) } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index b60abad5ed..6546f57ba6 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -112,10 +112,8 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw 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 } service.state.ContainerStatus[req.NetworkContainerid] = @@ -167,7 +165,7 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw case cns.KubernetesCRD: // Validate and Update the SecondaryIpConfig state - returnCode, returnMesage := service.updateIpConfigsStateUntransacted(req, existingSecondaryIPConfigs) + returnCode, returnMesage := service.updateIpConfigsStateUntransacted(req, existingNCStatus) if returnCode != 0 { return returnCode, returnMesage } @@ -197,14 +195,14 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw // This func will compute the deltaIpConfigState which needs to be updated (Added or Deleted) // from the inmemory map // Note: Also this func is an untransacted API as the caller will take a Service lock -func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateNetworkContainerRequest, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig) (int, string) { +func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateNetworkContainerRequest, existingNCStatus containerstatus) (int, string) { // parse the existingSecondaryIpConfigState to find the deleted Ips newIPConfigs := req.SecondaryIPConfigs var tobeDeletedIpConfigs = make(map[string]cns.SecondaryIPConfig) // Populate the ToBeDeleted list, Secondary IPs which doesnt exist in New request anymore. // We will later remove them from the in-memory cache - for secondaryIpId, existingIPConfig := range existingSecondaryIPConfigs { + for secondaryIpId, existingIPConfig := range existingNCStatus.CreateNetworkContainerRequest.SecondaryIPConfigs { _, exists := newIPConfigs[secondaryIpId] if !exists { // IP got removed in the updated request, add it in tobeDeletedIps @@ -237,7 +235,7 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN } // Add the newIpConfigs, ignore if ip state is already in the map - service.addIPConfigStateUntransacted(req.NCVersion, req.NetworkContainerid, newIPConfigs) + service.addIPConfigStateUntransacted(req.Version, existingNCStatus, newIPConfigs) return 0, "" } @@ -245,12 +243,12 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN // addIPConfigStateUntransacted adds the IPConfigs to the PodIpConfigState map with Available state // If the IP is already added then it will be an idempotent call. Also note, caller will // acquire/release the service lock. -func (service *HTTPRestService) addIPConfigStateUntransacted(ncVersion, ncId string, ipconfigs map[string]cns.SecondaryIPConfig) { +func (service *HTTPRestService) addIPConfigStateUntransacted(version string, existingNCStatus containerstatus, ipconfigs map[string]cns.SecondaryIPConfig) { - existingNCStatus, _ := service.state.ContainerStatus[ncId] - existingNCVersion, _ := strconv.Atoi(existingNCStatus.CreateNetworkContainerRequest.NCVersion) - nmAgentNCVersion := getNCVersionFromNMAgent(ncId) - newNCVersion, _ := strconv.Atoi(ncVersion) + var newNCVersion int + newNCVersion, _ = strconv.Atoi(version) + existingNCVersion, _ := strconv.Atoi(existingNCStatus.CreateNetworkContainerRequest.Version) + nmAgentNCVersion := getNCVersionFromNMAgent(existingNCStatus.ID) if nmAgentNCVersion >= newNCVersion { // add ipconfigs to state for ipId, ipconfig := range ipconfigs { @@ -261,7 +259,7 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(ncVersion, ncId str // add the new State ipconfigStatus := cns.IPConfigurationStatus{ - NCID: ncId, + NCID: existingNCStatus.ID, ID: ipId, IPAddress: ipconfig.IPAddress, State: cns.Available, @@ -272,13 +270,10 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(ncVersion, ncId str // Todo Update batch API and maintain the count - existingNCStatus.CreateNetworkContainerRequest.NCVersion = strconv.Itoa(newNCVersion) } } else { if nmAgentNCVersion >= existingNCVersion { // todo, update all pending program IP to available. - // Unconmment the following command when getNCVersionFromNMAgent function finished. - // existingNCStatus.CreateNetworkContainerRequest.NCVersion = strconv.Itoa(nmAgentNCVersion) } else { // todo, update IPs in ipconfigs which is not in the map to pending program. } From 0c117f1b37ce40317852252ed0b4d79f0598cf96 Mon Sep 17 00:00:00 2001 From: Shufang Date: Thu, 15 Oct 2020 20:13:27 -0700 Subject: [PATCH 05/15] Modify function parameter to meet current unit test requirement. --- cns/restserver/util.go | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 6546f57ba6..7affea2dff 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -112,8 +112,12 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw existingNCStatus, ok := service.state.ContainerStatus[req.NetworkContainerid] var hostVersion string + var existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig //uuid is key + var existingNCVersion string if ok { hostVersion = existingNCStatus.HostVersion + existingSecondaryIPConfigs = existingNCStatus.CreateNetworkContainerRequest.SecondaryIPConfigs + existingNCVersion = existingNCStatus.VMVersion } service.state.ContainerStatus[req.NetworkContainerid] = @@ -165,7 +169,7 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw case cns.KubernetesCRD: // Validate and Update the SecondaryIpConfig state - returnCode, returnMesage := service.updateIpConfigsStateUntransacted(req, existingNCStatus) + returnCode, returnMesage := service.updateIpConfigsStateUntransacted(req, existingSecondaryIPConfigs, existingNCVersion) if returnCode != 0 { return returnCode, returnMesage } @@ -195,14 +199,14 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw // This func will compute the deltaIpConfigState which needs to be updated (Added or Deleted) // from the inmemory map // Note: Also this func is an untransacted API as the caller will take a Service lock -func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateNetworkContainerRequest, existingNCStatus containerstatus) (int, string) { +func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateNetworkContainerRequest, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig, existingNCVersion string) (int, string) { // parse the existingSecondaryIpConfigState to find the deleted Ips newIPConfigs := req.SecondaryIPConfigs var tobeDeletedIpConfigs = make(map[string]cns.SecondaryIPConfig) // Populate the ToBeDeleted list, Secondary IPs which doesnt exist in New request anymore. // We will later remove them from the in-memory cache - for secondaryIpId, existingIPConfig := range existingNCStatus.CreateNetworkContainerRequest.SecondaryIPConfigs { + for secondaryIpId, existingIPConfig := range existingSecondaryIPConfigs { _, exists := newIPConfigs[secondaryIpId] if !exists { // IP got removed in the updated request, add it in tobeDeletedIps @@ -235,7 +239,7 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN } // Add the newIpConfigs, ignore if ip state is already in the map - service.addIPConfigStateUntransacted(req.Version, existingNCStatus, newIPConfigs) + service.addIPConfigStateUntransacted(req, existingNCVersion, newIPConfigs) return 0, "" } @@ -243,12 +247,13 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN // addIPConfigStateUntransacted adds the IPConfigs to the PodIpConfigState map with Available state // If the IP is already added then it will be an idempotent call. Also note, caller will // acquire/release the service lock. -func (service *HTTPRestService) addIPConfigStateUntransacted(version string, existingNCStatus containerstatus, ipconfigs map[string]cns.SecondaryIPConfig) { +func (service *HTTPRestService) addIPConfigStateUntransacted(req cns.CreateNetworkContainerRequest, existingNCVersionInString string, ipconfigs map[string]cns.SecondaryIPConfig) { var newNCVersion int - newNCVersion, _ = strconv.Atoi(version) - existingNCVersion, _ := strconv.Atoi(existingNCStatus.CreateNetworkContainerRequest.Version) - nmAgentNCVersion := getNCVersionFromNMAgent(existingNCStatus.ID) + var existingNCVersion int + newNCVersion, _ = strconv.Atoi(req.Version) + existingNCVersion, _ = strconv.Atoi(existingNCVersionInString) + nmAgentNCVersion := getNCVersionFromNMAgent(req.NetworkContainerid) if nmAgentNCVersion >= newNCVersion { // add ipconfigs to state for ipId, ipconfig := range ipconfigs { @@ -259,7 +264,7 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(version string, exi // add the new State ipconfigStatus := cns.IPConfigurationStatus{ - NCID: existingNCStatus.ID, + NCID: req.NetworkContainerid, ID: ipId, IPAddress: ipconfig.IPAddress, State: cns.Available, From 4a79902a3a779eebdd3d6440a270957bbe9dda9f Mon Sep 17 00:00:00 2001 From: Shufang Date: Sat, 17 Oct 2020 00:23:15 -0700 Subject: [PATCH 06/15] Add updating secondary IPs to pending programming status logic. --- cns/restserver/util.go | 43 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 7affea2dff..aa6e2c929d 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -279,8 +279,51 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(req cns.CreateNetwo } else { if nmAgentNCVersion >= existingNCVersion { // todo, update all pending program IP to available. + for ipId, ipconfig := range ipconfigs { + // if this IPConfig already exists in the map, then ignore as this is an idempotent state + if existingPodIpConfigStatus, exists := service.PodIPConfigState[ipId]; exists { + if existingPodIpConfigStatus.State == cns.PendingProgramming { + existingPodIpConfigStatus.State = cns.Available + } + continue + } + + // add the new State + ipconfigStatus := cns.IPConfigurationStatus{ + NCID: req.NetworkContainerid, + ID: ipId, + IPAddress: ipconfig.IPAddress, + State: cns.PendingProgramming, + OrchestratorContext: nil, + } + + service.PodIPConfigState[ipId] = ipconfigStatus + + // Todo Update batch API and maintain the count + + } } else { // todo, update IPs in ipconfigs which is not in the map to pending program. + for ipId, ipconfig := range ipconfigs { + // if this IPConfig already exists in the map, then ignore as this is an idempotent state + if _, exists := service.PodIPConfigState[ipId]; exists { + continue + } + + // add the new State + ipconfigStatus := cns.IPConfigurationStatus{ + NCID: req.NetworkContainerid, + ID: ipId, + IPAddress: ipconfig.IPAddress, + State: cns.PendingProgramming, + OrchestratorContext: nil, + } + + service.PodIPConfigState[ipId] = ipconfigStatus + + // Todo Update batch API and maintain the count + + } } } } From 6cdb6f9c39c5fd2b54a7880c68a2c37b33df8b3f Mon Sep 17 00:00:00 2001 From: Shufang Date: Mon, 19 Oct 2020 00:34:12 -0700 Subject: [PATCH 07/15] Add ip state update when CRD status change. --- cns/restserver/util.go | 120 +++++++++++++++-------------------------- 1 file changed, 43 insertions(+), 77 deletions(-) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index aa6e2c929d..ebf6f62bad 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -118,6 +118,12 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw hostVersion = existingNCStatus.HostVersion existingSecondaryIPConfigs = existingNCStatus.CreateNetworkContainerRequest.SecondaryIPConfigs existingNCVersion = existingNCStatus.VMVersion + } else { + // Host version is the NC version from NMAgent, set it -2 to indicate no result from NMAgent yet. + // Existing NC status will be set as -1 to indicate no version exist yet. + // Intentionally set default host version smaller than existing NC version. + hostVersion = "-2" + existingNCVersion = "-1" } service.state.ContainerStatus[req.NetworkContainerid] = @@ -196,10 +202,9 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw return 0, "" } -// This func will compute the deltaIpConfigState which needs to be updated (Added or Deleted) -// from the inmemory map +// This func will compute the deltaIpConfigState which needs to be updated (Added or Deleted) from the inmemory map // Note: Also this func is an untransacted API as the caller will take a Service lock -func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateNetworkContainerRequest, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig, existingNCVersion string) (int, string) { +func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateNetworkContainerRequest, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig, existingNCVersionInString string) (int, string) { // parse the existingSecondaryIpConfigState to find the deleted Ips newIPConfigs := req.SecondaryIPConfigs var tobeDeletedIpConfigs = make(map[string]cns.SecondaryIPConfig) @@ -238,93 +243,54 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN } } - // Add the newIpConfigs, ignore if ip state is already in the map - service.addIPConfigStateUntransacted(req, existingNCVersion, newIPConfigs) - - return 0, "" -} - -// addIPConfigStateUntransacted adds the IPConfigs to the PodIpConfigState map with Available state -// If the IP is already added then it will be an idempotent call. Also note, caller will -// acquire/release the service lock. -func (service *HTTPRestService) addIPConfigStateUntransacted(req cns.CreateNetworkContainerRequest, existingNCVersionInString string, ipconfigs map[string]cns.SecondaryIPConfig) { - + // Add the newIpConfigs for 3 different conditions + // 1. If NC version from NMAgent is larger or same as new NC version, add new IP as available. + // 2. If NC version from NMAgent is larger or same as existing NC version, set existing pending programming IP as available and add new IP as pending programming. + // 3. If NC version from NMAgent is smaller than existing NC version, add new IP as pending programming. var newNCVersion int var existingNCVersion int newNCVersion, _ = strconv.Atoi(req.Version) existingNCVersion, _ = strconv.Atoi(existingNCVersionInString) + // Wait for Ashvin's thread update which querying NMAgent to get latest NC version and save it locally. nmAgentNCVersion := getNCVersionFromNMAgent(req.NetworkContainerid) - - if nmAgentNCVersion >= newNCVersion { // add ipconfigs to state - for ipId, ipconfig := range ipconfigs { - // if this IPConfig already exists in the map, then ignore as this is an idempotent state - if _, exists := service.PodIPConfigState[ipId]; exists { - continue - } - - // add the new State - ipconfigStatus := cns.IPConfigurationStatus{ - NCID: req.NetworkContainerid, - ID: ipId, - IPAddress: ipconfig.IPAddress, - State: cns.Available, - OrchestratorContext: nil, - } - - service.PodIPConfigState[ipId] = ipconfigStatus - - // Todo Update batch API and maintain the count - - } + if nmAgentNCVersion >= newNCVersion { + service.addIPConfigStateUntransacted(cns.Available, cns.Available, newIPConfigs) } else { if nmAgentNCVersion >= existingNCVersion { - // todo, update all pending program IP to available. - for ipId, ipconfig := range ipconfigs { - // if this IPConfig already exists in the map, then ignore as this is an idempotent state - if existingPodIpConfigStatus, exists := service.PodIPConfigState[ipId]; exists { - if existingPodIpConfigStatus.State == cns.PendingProgramming { - existingPodIpConfigStatus.State = cns.Available - } - continue - } - - // add the new State - ipconfigStatus := cns.IPConfigurationStatus{ - NCID: req.NetworkContainerid, - ID: ipId, - IPAddress: ipconfig.IPAddress, - State: cns.PendingProgramming, - OrchestratorContext: nil, - } - - service.PodIPConfigState[ipId] = ipconfigStatus - - // Todo Update batch API and maintain the count - - } + service.addIPConfigStateUntransacted(cns.Available, cns.PendingProgramming, newIPConfigs) } else { - // todo, update IPs in ipconfigs which is not in the map to pending program. - for ipId, ipconfig := range ipconfigs { - // if this IPConfig already exists in the map, then ignore as this is an idempotent state - if _, exists := service.PodIPConfigState[ipId]; exists { - continue - } - - // add the new State - ipconfigStatus := cns.IPConfigurationStatus{ - NCID: req.NetworkContainerid, - ID: ipId, - IPAddress: ipconfig.IPAddress, - State: cns.PendingProgramming, - OrchestratorContext: nil, - } + service.addIPConfigStateUntransacted(cns.PendingProgramming, cns.PendingProgramming, newIPConfigs) + } + } - service.PodIPConfigState[ipId] = ipconfigStatus + service.state.ContainerStatus[req.NetworkContainerid].HostVersion = nmAgentNCVersion - // Todo Update batch API and maintain the count + return 0, "" +} +// addIPConfigStateUntransacted adds the IPConfigs to the PodIpConfigState map with Available state +// If the IP is already added then it will be an idempotent call. Also note, caller will +// acquire/release the service lock. +func (service *HTTPRestService) addIPConfigStateUntransacted(updatedExistingIPCNSStatus, newIPCNSStatus string, ipconfigs map[string]cns.SecondaryIPConfig) { + // add ipconfigs to state + for ipId, ipconfig := range ipconfigs { + if existingPodIpConfigStatus, exists := service.PodIPConfigState[ipId]; exists { + if existingPodIpConfigStatus.State == cns.PendingProgramming { + existingPodIpConfigStatus.State = updatedExistingIPCNSStatus } + continue + } + ipconfigStatus := cns.IPConfigurationStatus{ + NCID: req.NetworkContainerid, + ID: ipId, + IPAddress: ipconfig.IPAddress, + State: newIPCNSStatus, + OrchestratorContext: nil, } + + service.PodIPConfigState[ipId] = ipconfigStatus + + // Todo Update batch API and maintain the count } } From a37b5e758ad4a3df5a1d4f9b4c50bf48bd21d0fa Mon Sep 17 00:00:00 2001 From: Shufang Date: Mon, 19 Oct 2020 00:41:30 -0700 Subject: [PATCH 08/15] Update NC ID. --- cns/restserver/util.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index ebf6f62bad..6cfecc59a3 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -254,12 +254,12 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN // Wait for Ashvin's thread update which querying NMAgent to get latest NC version and save it locally. nmAgentNCVersion := getNCVersionFromNMAgent(req.NetworkContainerid) if nmAgentNCVersion >= newNCVersion { - service.addIPConfigStateUntransacted(cns.Available, cns.Available, newIPConfigs) + service.addIPConfigStateUntransacted(cns.Available, cns.Available, req.NetworkContainerid, newIPConfigs) } else { if nmAgentNCVersion >= existingNCVersion { - service.addIPConfigStateUntransacted(cns.Available, cns.PendingProgramming, newIPConfigs) + service.addIPConfigStateUntransacted(cns.Available, cns.PendingProgramming, req.NetworkContainerid, newIPConfigs) } else { - service.addIPConfigStateUntransacted(cns.PendingProgramming, cns.PendingProgramming, newIPConfigs) + service.addIPConfigStateUntransacted(cns.PendingProgramming, cns.PendingProgramming, req.NetworkContainerid, newIPConfigs) } } @@ -271,7 +271,7 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN // addIPConfigStateUntransacted adds the IPConfigs to the PodIpConfigState map with Available state // If the IP is already added then it will be an idempotent call. Also note, caller will // acquire/release the service lock. -func (service *HTTPRestService) addIPConfigStateUntransacted(updatedExistingIPCNSStatus, newIPCNSStatus string, ipconfigs map[string]cns.SecondaryIPConfig) { +func (service *HTTPRestService) addIPConfigStateUntransacted(updatedExistingIPCNSStatus, newIPCNSStatus, ncId string, ipconfigs map[string]cns.SecondaryIPConfig) { // add ipconfigs to state for ipId, ipconfig := range ipconfigs { if existingPodIpConfigStatus, exists := service.PodIPConfigState[ipId]; exists { @@ -281,7 +281,7 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(updatedExistingIPCN continue } ipconfigStatus := cns.IPConfigurationStatus{ - NCID: req.NetworkContainerid, + NCID: ncId, ID: ipId, IPAddress: ipconfig.IPAddress, State: newIPCNSStatus, From 1056109bac4c356a604a741e2ad24dcd5b256868 Mon Sep 17 00:00:00 2001 From: Shufang Date: Mon, 19 Oct 2020 00:41:30 -0700 Subject: [PATCH 09/15] Update NC ID. --- cns/restserver/util.go | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index ebf6f62bad..15ba71b38a 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -191,13 +191,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, "" } @@ -254,16 +247,19 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN // Wait for Ashvin's thread update which querying NMAgent to get latest NC version and save it locally. nmAgentNCVersion := getNCVersionFromNMAgent(req.NetworkContainerid) if nmAgentNCVersion >= newNCVersion { - service.addIPConfigStateUntransacted(cns.Available, cns.Available, newIPConfigs) + service.addIPConfigStateUntransacted(cns.Available, cns.Available, req.NetworkContainerid, newIPConfigs) } else { if nmAgentNCVersion >= existingNCVersion { - service.addIPConfigStateUntransacted(cns.Available, cns.PendingProgramming, newIPConfigs) + service.addIPConfigStateUntransacted(cns.Available, cns.PendingProgramming, req.NetworkContainerid, newIPConfigs) } else { - service.addIPConfigStateUntransacted(cns.PendingProgramming, cns.PendingProgramming, newIPConfigs) + service.addIPConfigStateUntransacted(cns.PendingProgramming, cns.PendingProgramming, req.NetworkContainerid, newIPConfigs) } } - service.state.ContainerStatus[req.NetworkContainerid].HostVersion = nmAgentNCVersion + existingNCStatus, ok := service.state.ContainerStatus[req.NetworkContainerid] + if ok { + existingNCStatus.HostVersion = strconv.Itoa(nmAgentNCVersion) + } return 0, "" } @@ -271,7 +267,7 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN // addIPConfigStateUntransacted adds the IPConfigs to the PodIpConfigState map with Available state // If the IP is already added then it will be an idempotent call. Also note, caller will // acquire/release the service lock. -func (service *HTTPRestService) addIPConfigStateUntransacted(updatedExistingIPCNSStatus, newIPCNSStatus string, ipconfigs map[string]cns.SecondaryIPConfig) { +func (service *HTTPRestService) addIPConfigStateUntransacted(updatedExistingIPCNSStatus, newIPCNSStatus, ncId string, ipconfigs map[string]cns.SecondaryIPConfig) { // add ipconfigs to state for ipId, ipconfig := range ipconfigs { if existingPodIpConfigStatus, exists := service.PodIPConfigState[ipId]; exists { @@ -281,7 +277,7 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(updatedExistingIPCN continue } ipconfigStatus := cns.IPConfigurationStatus{ - NCID: req.NetworkContainerid, + NCID: ncId, ID: ipId, IPAddress: ipconfig.IPAddress, State: newIPCNSStatus, From 41e0b9cc0f40debc339e741eb7ffcf0383d2288a Mon Sep 17 00:00:00 2001 From: Shufang Date: Mon, 19 Oct 2020 01:28:41 -0700 Subject: [PATCH 10/15] Update comments. --- cns/restserver/util.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 15ba71b38a..f13823d6de 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -276,6 +276,7 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(updatedExistingIPCN } continue } + // add the new State ipconfigStatus := cns.IPConfigurationStatus{ NCID: ncId, ID: ipId, From 7e2190c3802cd906e49404d8581baee62faaa34d Mon Sep 17 00:00:00 2001 From: Shufang Date: Mon, 19 Oct 2020 15:47:08 -0700 Subject: [PATCH 11/15] Remove NC version compare and update logic. This logic will be moved to a background thread. --- cns/restserver/util.go | 49 ++++++------------------------------------ 1 file changed, 7 insertions(+), 42 deletions(-) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index f13823d6de..61a7788b6d 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -7,7 +7,6 @@ import ( "encoding/json" "fmt" "io/ioutil" - "math" "net/http" "strconv" "time" @@ -113,17 +112,14 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw existingNCStatus, ok := service.state.ContainerStatus[req.NetworkContainerid] var hostVersion string var existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig //uuid is key - var existingNCVersion string if ok { hostVersion = existingNCStatus.HostVersion existingSecondaryIPConfigs = existingNCStatus.CreateNetworkContainerRequest.SecondaryIPConfigs - existingNCVersion = existingNCStatus.VMVersion } else { // Host version is the NC version from NMAgent, set it -2 to indicate no result from NMAgent yet. // Existing NC status will be set as -1 to indicate no version exist yet. // Intentionally set default host version smaller than existing NC version. - hostVersion = "-2" - existingNCVersion = "-1" + hostVersion = "-1" } service.state.ContainerStatus[req.NetworkContainerid] = @@ -175,7 +171,7 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw case cns.KubernetesCRD: // Validate and Update the SecondaryIpConfig state - returnCode, returnMesage := service.updateIpConfigsStateUntransacted(req, existingSecondaryIPConfigs, existingNCVersion) + returnCode, returnMesage := service.updateIpConfigsStateUntransacted(req, existingSecondaryIPConfigs) if returnCode != 0 { return returnCode, returnMesage } @@ -197,7 +193,7 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw // This func will compute the deltaIpConfigState which needs to be updated (Added or Deleted) from the inmemory map // Note: Also this func is an untransacted API as the caller will take a Service lock -func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateNetworkContainerRequest, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig, existingNCVersionInString string) (int, string) { +func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateNetworkContainerRequest, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig) (int, string) { // parse the existingSecondaryIpConfigState to find the deleted Ips newIPConfigs := req.SecondaryIPConfigs var tobeDeletedIpConfigs = make(map[string]cns.SecondaryIPConfig) @@ -236,30 +232,7 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN } } - // Add the newIpConfigs for 3 different conditions - // 1. If NC version from NMAgent is larger or same as new NC version, add new IP as available. - // 2. If NC version from NMAgent is larger or same as existing NC version, set existing pending programming IP as available and add new IP as pending programming. - // 3. If NC version from NMAgent is smaller than existing NC version, add new IP as pending programming. - var newNCVersion int - var existingNCVersion int - newNCVersion, _ = strconv.Atoi(req.Version) - existingNCVersion, _ = strconv.Atoi(existingNCVersionInString) - // Wait for Ashvin's thread update which querying NMAgent to get latest NC version and save it locally. - nmAgentNCVersion := getNCVersionFromNMAgent(req.NetworkContainerid) - if nmAgentNCVersion >= newNCVersion { - service.addIPConfigStateUntransacted(cns.Available, cns.Available, req.NetworkContainerid, newIPConfigs) - } else { - if nmAgentNCVersion >= existingNCVersion { - service.addIPConfigStateUntransacted(cns.Available, cns.PendingProgramming, req.NetworkContainerid, newIPConfigs) - } else { - service.addIPConfigStateUntransacted(cns.PendingProgramming, cns.PendingProgramming, req.NetworkContainerid, newIPConfigs) - } - } - - existingNCStatus, ok := service.state.ContainerStatus[req.NetworkContainerid] - if ok { - existingNCStatus.HostVersion = strconv.Itoa(nmAgentNCVersion) - } + service.addIPConfigStateUntransacted(cns.Available, req.NetworkContainerid, newIPConfigs) return 0, "" } @@ -267,13 +240,10 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN // addIPConfigStateUntransacted adds the IPConfigs to the PodIpConfigState map with Available state // If the IP is already added then it will be an idempotent call. Also note, caller will // acquire/release the service lock. -func (service *HTTPRestService) addIPConfigStateUntransacted(updatedExistingIPCNSStatus, newIPCNSStatus, ncId string, ipconfigs map[string]cns.SecondaryIPConfig) { +func (service *HTTPRestService) addIPConfigStateUntransacted(newIPCNSStatus, ncId string, ipconfigs map[string]cns.SecondaryIPConfig) { // add ipconfigs to state for ipId, ipconfig := range ipconfigs { - if existingPodIpConfigStatus, exists := service.PodIPConfigState[ipId]; exists { - if existingPodIpConfigStatus.State == cns.PendingProgramming { - existingPodIpConfigStatus.State = updatedExistingIPCNSStatus - } + if _, exists := service.PodIPConfigState[ipId]; exists { continue } // add the new State @@ -284,6 +254,7 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(updatedExistingIPCN State: newIPCNSStatus, OrchestratorContext: nil, } + logger.Printf("[Azure-Cns] Add IP %s as %s", ipconfig.IPAddress, newIPCNSStatus) service.PodIPConfigState[ipId] = ipconfigStatus @@ -291,12 +262,6 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(updatedExistingIPCN } } -// To do, complete this logic according to getNetworkContainerStatus -func getNCVersionFromNMAgent(ncid string) int { - // return a largest int to not break current. - return math.MaxInt64 -} - // Todo: call this when request is received func validateIPSubnet(ipSubnet cns.IPSubnet) error { if ipSubnet.IPAddress == "" { From 4b28926e84bb95ae87999c6509544cdc100e5725 Mon Sep 17 00:00:00 2001 From: Shufang Date: Mon, 19 Oct 2020 16:46:19 -0700 Subject: [PATCH 12/15] Update comments accordingly. --- 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 61a7788b6d..3e65d01e13 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -116,9 +116,7 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw hostVersion = existingNCStatus.HostVersion existingSecondaryIPConfigs = existingNCStatus.CreateNetworkContainerRequest.SecondaryIPConfigs } else { - // Host version is the NC version from NMAgent, set it -2 to indicate no result from NMAgent yet. - // Existing NC status will be set as -1 to indicate no version exist yet. - // Intentionally set default host version smaller than existing NC version. + // Host version is the NC version from NMAgent, set it -1 to indicate no result from NMAgent yet. hostVersion = "-1" } From 895fbf61c06e62e8e8c9f2f7a47e6faac68c3a59 Mon Sep 17 00:00:00 2001 From: Shufang Date: Tue, 20 Oct 2020 16:55:43 -0700 Subject: [PATCH 13/15] Add unit test. --- cns/fakes/imdsclientfake.go | 8 ++++++ cns/imdsclient/api.go | 1 + cns/imdsclient/imdsclient.go | 9 +++++++ cns/restserver/internalapi_test.go | 40 +++++++++++++++++++++--------- cns/restserver/ipam_test.go | 2 +- cns/restserver/util.go | 16 +++++++++--- 6 files changed, 60 insertions(+), 16 deletions(-) diff --git a/cns/fakes/imdsclientfake.go b/cns/fakes/imdsclientfake.go index e2e9413af8..4468a3e5e8 100644 --- a/cns/fakes/imdsclientfake.go +++ b/cns/fakes/imdsclientfake.go @@ -47,3 +47,11 @@ func (imdsClient *ImdsClientTest) GetPrimaryInterfaceInfoFromMemory() (*imdsclie return imdsClient.GetPrimaryInterfaceInfoFromHost() } + +// GetNMagentVersion - Mock implementation to return host NMAgent NC version +// Set it as 0 which is the same as default initial NC version for testing purpose +func (imdsClient *ImdsClientTest) GetNMagentVersion() int { + logger.Printf("[Azure CNS] GetNMagentVersionFromNMAgent") + + return 0 +} diff --git a/cns/imdsclient/api.go b/cns/imdsclient/api.go index 0af765f07c..20e7fb23c3 100644 --- a/cns/imdsclient/api.go +++ b/cns/imdsclient/api.go @@ -74,4 +74,5 @@ type ImdsClientInterface interface { GetNetworkContainerInfoFromHost(networkContainerID string, primaryAddress string, authToken string, apiVersion string) (*ContainerVersion, error) GetPrimaryInterfaceInfoFromHost() (*InterfaceInfo, error) GetPrimaryInterfaceInfoFromMemory() (*InterfaceInfo, error) + GetNMagentVersion() int } diff --git a/cns/imdsclient/imdsclient.go b/cns/imdsclient/imdsclient.go index 6d2ab911b1..21834bf04c 100644 --- a/cns/imdsclient/imdsclient.go +++ b/cns/imdsclient/imdsclient.go @@ -7,6 +7,7 @@ import ( "encoding/json" "encoding/xml" "fmt" + "math" "net/http" "strings" @@ -132,3 +133,11 @@ func (imdsClient *ImdsClient) GetPrimaryInterfaceInfoFromMemory() (*InterfaceInf return iface, err } + +// GetNMagentVersion is a temp implementation which will be removed once background thread +// updating host version is ready. Return max integer value to regress current AKS scenario +func (imdsClient *ImdsClient) GetNMagentVersion() int { + logger.Printf("[Azure CNS] GetNMagentVersionFromNMAgent") + + return math.MaxInt64 +} diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index b6721bf8d9..babfea3358 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -35,8 +35,17 @@ func TestCreateOrUpdateNetworkContainerInternal(t *testing.T) { setEnv(t) setOrchestratorTypeInternal(cns.KubernetesCRD) + // NC version set as 0 which is the default initial value. + validateCreateOrUpdateNCInternal(t, 2, "0") +} + +func TestCreateOrUpdateNCWithLargerVersionComparedToNMAgent(t *testing.T) { + restartService() - validateCreateOrUpdateNCInternal(t, 2) + setEnv(t) + setOrchestratorTypeInternal(cns.KubernetesCRD) + // NC version set as 1 which is larger than NC version get from mock nmagent. + validateCreateOrUpdateNCInternal(t, 2, "1") } func TestReconcileNCWithEmptyState(t *testing.T) { @@ -69,7 +78,7 @@ func TestReconcileNCWithExistingState(t *testing.T) { secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ } - req := generateNetworkContainerRequest(secondaryIPConfigs, "reconcileNc1") + req := generateNetworkContainerRequest(secondaryIPConfigs, "reconcileNc1", "0") expectedAllocatedPods := make(map[string]cns.KubernetesPodInfo) expectedAllocatedPods["10.0.0.6"] = cns.KubernetesPodInfo{ @@ -106,7 +115,7 @@ func TestReconcileNCWithSystemPods(t *testing.T) { secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ } - req := generateNetworkContainerRequest(secondaryIPConfigs, uuid.New().String()) + req := generateNetworkContainerRequest(secondaryIPConfigs, uuid.New().String(), "0") expectedAllocatedPods := make(map[string]cns.KubernetesPodInfo) expectedAllocatedPods["10.0.0.6"] = cns.KubernetesPodInfo{ @@ -135,7 +144,7 @@ func setOrchestratorTypeInternal(orchestratorType string) { svc.state.OrchestratorType = orchestratorType } -func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int) { +func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int, ncVersion string) { secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig) ncId := "testNc1" @@ -148,7 +157,7 @@ func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int) { startingIndex++ } - createAndValidateNCRequest(t, secondaryIPConfigs, ncId) + createAndValidateNCRequest(t, secondaryIPConfigs, ncId, ncVersion) // now Validate Update, add more secondaryIpConfig and it should handle the update fmt.Println("Validate Scaleup") @@ -160,7 +169,7 @@ func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int) { startingIndex++ } - createAndValidateNCRequest(t, secondaryIPConfigs, ncId) + createAndValidateNCRequest(t, secondaryIPConfigs, ncId, ncVersion) // now Scale down, delete 3 ipaddresses from secondaryIpConfig req fmt.Println("Validate Scale down") @@ -174,7 +183,7 @@ func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int) { } } - createAndValidateNCRequest(t, secondaryIPConfigs, ncId) + createAndValidateNCRequest(t, secondaryIPConfigs, ncId, ncVersion) // Cleanup all SecondaryIps fmt.Println("Validate no SecondaryIpconfigs") @@ -182,11 +191,11 @@ func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int) { delete(secondaryIPConfigs, ipid) } - createAndValidateNCRequest(t, secondaryIPConfigs, ncId) + createAndValidateNCRequest(t, secondaryIPConfigs, ncId, ncVersion) } -func createAndValidateNCRequest(t *testing.T, secondaryIPConfigs map[string]cns.SecondaryIPConfig, ncId string) { - req := generateNetworkContainerRequest(secondaryIPConfigs, ncId) +func createAndValidateNCRequest(t *testing.T, secondaryIPConfigs map[string]cns.SecondaryIPConfig, ncId, ncVersion string) { + req := generateNetworkContainerRequest(secondaryIPConfigs, ncId, ncVersion) returnCode := svc.CreateOrUpdateNetworkContainerInternal(req, fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) if returnCode != 0 { t.Fatalf("Failed to createNetworkContainerRequest, req: %+v, err: %d", req, returnCode) @@ -216,6 +225,12 @@ func validateNetworkRequest(t *testing.T, req cns.CreateNetworkContainerRequest) t.Fatalf("Failed as Secondary IP count doesnt match in PodIpConfig state, expected:%d, actual %d", len(req.SecondaryIPConfigs), len(svc.PodIPConfigState)) } + var expectedIPStatus string + if containerStatus.VMVersion > containerStatus.HostVersion { + expectedIPStatus = cns.PendingProgramming + } else { + expectedIPStatus = cns.Available + } var alreadyValidated = make(map[string]string) for ipid, ipStatus := range svc.PodIPConfigState { if ipaddress, found := alreadyValidated[ipid]; !found { @@ -240,7 +255,7 @@ func validateNetworkRequest(t *testing.T, req cns.CreateNetworkContainerRequest) } else { t.Fatalf("Failed to find podContext for allocated ip: %+v, podinfo :%+v", ipStatus, podInfo) } - } else if ipStatus.State != cns.Available { + } else if ipStatus.State != expectedIPStatus { // Todo: Validate for pendingRelease as well t.Fatalf("IPId: %s State is not Available, ipStatus: %+v", ipid, ipStatus) } @@ -256,7 +271,7 @@ func validateNetworkRequest(t *testing.T, req cns.CreateNetworkContainerRequest) } } -func generateNetworkContainerRequest(secondaryIps map[string]cns.SecondaryIPConfig, ncId string) cns.CreateNetworkContainerRequest { +func generateNetworkContainerRequest(secondaryIps map[string]cns.SecondaryIPConfig, ncId string, ncVersion string) cns.CreateNetworkContainerRequest { var ipConfig cns.IPConfiguration ipConfig.DNSServers = dnsservers ipConfig.GatewayIPAddress = gatewayIp @@ -269,6 +284,7 @@ func generateNetworkContainerRequest(secondaryIps map[string]cns.SecondaryIPConf NetworkContainerType: dockerContainerType, NetworkContainerid: ncId, IPConfiguration: ipConfig, + Version: ncVersion, } req.SecondaryIPConfigs = make(map[string]cns.SecondaryIPConfig) diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 1d072c4666..f1a280eea3 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -144,7 +144,7 @@ func UpdatePodIpConfigState(t *testing.T, svc *HTTPRestService, ipconfigs map[st secondaryIPConfigs[ipId] = secIpConfig } - createAndValidateNCRequest(t, secondaryIPConfigs, testNCID) + createAndValidateNCRequest(t, secondaryIPConfigs, testNCID, "0") // update ipconfigs to expected state for ipId, ipconfig := range ipconfigs { diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 3e65d01e13..6d62c28d95 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -169,7 +169,7 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw case cns.KubernetesCRD: // Validate and Update the SecondaryIpConfig state - returnCode, returnMesage := service.updateIpConfigsStateUntransacted(req, existingSecondaryIPConfigs) + returnCode, returnMesage := service.updateIpConfigsStateUntransacted(req, existingSecondaryIPConfigs, hostVersion) if returnCode != 0 { return returnCode, returnMesage } @@ -191,7 +191,7 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw // This func will compute the deltaIpConfigState which needs to be updated (Added or Deleted) from the inmemory map // Note: Also this func is an untransacted API as the caller will take a Service lock -func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateNetworkContainerRequest, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig) (int, string) { +func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateNetworkContainerRequest, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig, hostVersion string) (int, string) { // parse the existingSecondaryIpConfigState to find the deleted Ips newIPConfigs := req.SecondaryIPConfigs var tobeDeletedIpConfigs = make(map[string]cns.SecondaryIPConfig) @@ -230,7 +230,17 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN } } - service.addIPConfigStateUntransacted(cns.Available, req.NetworkContainerid, newIPConfigs) + newNCVersion, _ := strconv.Atoi(req.Version) + nmagentNCVersion, _ := strconv.Atoi(hostVersion) + + // TODO, remove this override when background thread which update nmagent version is ready. + nmagentNCVersion = service.imdsClient.GetNMagentVersion() + + if nmagentNCVersion >= newNCVersion { + service.addIPConfigStateUntransacted(cns.Available, req.NetworkContainerid, newIPConfigs) + } else { + service.addIPConfigStateUntransacted(cns.PendingProgramming, req.NetworkContainerid, newIPConfigs) + } return 0, "" } From 8afcf44d9c3a33f48feab218da3a35a867fb41c3 Mon Sep 17 00:00:00 2001 From: Shufang Date: Tue, 20 Oct 2020 17:32:47 -0700 Subject: [PATCH 14/15] Update nmagent version in test. --- cns/restserver/internalapi_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index babfea3358..d3cb462402 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -226,11 +226,13 @@ func validateNetworkRequest(t *testing.T, req cns.CreateNetworkContainerRequest) } var expectedIPStatus string - if containerStatus.VMVersion > containerStatus.HostVersion { + // 0 is the default NMAgent version return from fake GetNMagentVersion + if containerStatus.VMVersion > "0" { expectedIPStatus = cns.PendingProgramming } else { expectedIPStatus = cns.Available } + t.Logf("VMVersion is %s, HostVersion is %s", containerStatus.VMVersion, containerStatus.HostVersion) var alreadyValidated = make(map[string]string) for ipid, ipStatus := range svc.PodIPConfigState { if ipaddress, found := alreadyValidated[ipid]; !found { @@ -257,7 +259,7 @@ func validateNetworkRequest(t *testing.T, req cns.CreateNetworkContainerRequest) } } else if ipStatus.State != expectedIPStatus { // Todo: Validate for pendingRelease as well - t.Fatalf("IPId: %s State is not Available, ipStatus: %+v", ipid, ipStatus) + t.Fatalf("IPId: %s State is not as expected, ipStatus is : %+v, expected status is %+v", ipid, ipStatus.State, expectedIPStatus) } alreadyValidated[ipid] = ipStatus.IPAddress From 5cf37b87c9c5cde10feb3ab5f7035d2ba397a82a Mon Sep 17 00:00:00 2001 From: Shufang Date: Thu, 22 Oct 2020 12:32:05 -0700 Subject: [PATCH 15/15] Update function name. --- cns/fakes/imdsclientfake.go | 8 ++++---- cns/imdsclient/api.go | 2 +- cns/imdsclient/imdsclient.go | 4 ++-- cns/restserver/internalapi_test.go | 2 +- cns/restserver/util.go | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cns/fakes/imdsclientfake.go b/cns/fakes/imdsclientfake.go index 4468a3e5e8..29655d26be 100644 --- a/cns/fakes/imdsclientfake.go +++ b/cns/fakes/imdsclientfake.go @@ -21,7 +21,7 @@ func NewFakeImdsClient() *ImdsClientTest { return &ImdsClientTest{} } -// GetNetworkContainerInfoFromHost- Mock implementation to return Container version info. +// GetNetworkContainerInfoFromHost - Mock implementation to return Container version info. func (imdsClient *ImdsClientTest) GetNetworkContainerInfoFromHost(networkContainerID string, primaryAddress string, authToken string, apiVersion string) (*imdsclient.ContainerVersion, error) { ret := &imdsclient.ContainerVersion{} @@ -48,10 +48,10 @@ func (imdsClient *ImdsClientTest) GetPrimaryInterfaceInfoFromMemory() (*imdsclie return imdsClient.GetPrimaryInterfaceInfoFromHost() } -// GetNMagentVersion - Mock implementation to return host NMAgent NC version +// GetNetworkContainerInfoFromHostWithoutToken - Mock implementation to return host NMAgent NC version // Set it as 0 which is the same as default initial NC version for testing purpose -func (imdsClient *ImdsClientTest) GetNMagentVersion() int { - logger.Printf("[Azure CNS] GetNMagentVersionFromNMAgent") +func (imdsClient *ImdsClientTest) GetNetworkContainerInfoFromHostWithoutToken() int { + logger.Printf("[Azure CNS] get the NC version from NMAgent") return 0 } diff --git a/cns/imdsclient/api.go b/cns/imdsclient/api.go index 20e7fb23c3..bc85f0c3b6 100644 --- a/cns/imdsclient/api.go +++ b/cns/imdsclient/api.go @@ -74,5 +74,5 @@ type ImdsClientInterface interface { GetNetworkContainerInfoFromHost(networkContainerID string, primaryAddress string, authToken string, apiVersion string) (*ContainerVersion, error) GetPrimaryInterfaceInfoFromHost() (*InterfaceInfo, error) GetPrimaryInterfaceInfoFromMemory() (*InterfaceInfo, error) - GetNMagentVersion() int + GetNetworkContainerInfoFromHostWithoutToken() int } diff --git a/cns/imdsclient/imdsclient.go b/cns/imdsclient/imdsclient.go index 21834bf04c..56f7d48751 100644 --- a/cns/imdsclient/imdsclient.go +++ b/cns/imdsclient/imdsclient.go @@ -134,9 +134,9 @@ func (imdsClient *ImdsClient) GetPrimaryInterfaceInfoFromMemory() (*InterfaceInf return iface, err } -// GetNMagentVersion is a temp implementation which will be removed once background thread +// GetNetworkContainerInfoFromHostWithoutToken is a temp implementation which will be removed once background thread // updating host version is ready. Return max integer value to regress current AKS scenario -func (imdsClient *ImdsClient) GetNMagentVersion() int { +func (imdsClient *ImdsClient) GetNetworkContainerInfoFromHostWithoutToken() int { logger.Printf("[Azure CNS] GetNMagentVersionFromNMAgent") return math.MaxInt64 diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index d3cb462402..923cc46032 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -226,7 +226,7 @@ func validateNetworkRequest(t *testing.T, req cns.CreateNetworkContainerRequest) } var expectedIPStatus string - // 0 is the default NMAgent version return from fake GetNMagentVersion + // 0 is the default NMAgent version return from fake GetNetworkContainerInfoFromHost if containerStatus.VMVersion > "0" { expectedIPStatus = cns.PendingProgramming } else { diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 6d62c28d95..e5eada3bf2 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -234,7 +234,7 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN nmagentNCVersion, _ := strconv.Atoi(hostVersion) // TODO, remove this override when background thread which update nmagent version is ready. - nmagentNCVersion = service.imdsClient.GetNMagentVersion() + nmagentNCVersion = service.imdsClient.GetNetworkContainerInfoFromHostWithoutToken() if nmagentNCVersion >= newNCVersion { service.addIPConfigStateUntransacted(cns.Available, req.NetworkContainerid, newIPConfigs)