From 37d4948518855875ee698d8d6ff28c07094ba2e6 Mon Sep 17 00:00:00 2001 From: Shufang Date: Mon, 26 Oct 2020 23:30:03 -0700 Subject: [PATCH 01/12] Add a go routine to update NC host version from NMAgent periodically. If orchestrator type is CRD, update pending programming IPs as well. --- cns/api.go | 1 + cns/configuration/configuration.go | 17 +++---- cns/fakes/cnsfake.go | 5 ++ cns/restserver/internalapi.go | 32 +++++++++++++ cns/restserver/internalapi_test.go | 75 ++++++++++++++++++++++++++++++ cns/restserver/ipam.go | 19 ++++++++ cns/restserver/util.go | 3 -- cns/service/main.go | 8 ++++ 8 files changed, 149 insertions(+), 11 deletions(-) diff --git a/cns/api.go b/cns/api.go index 61d8ec82e3..833fef4da4 100644 --- a/cns/api.go +++ b/cns/api.go @@ -38,6 +38,7 @@ type HTTPService interface { SendNCSnapShotPeriodically(int, chan bool) SetNodeOrchestrator(*SetOrchestratorTypeRequest) SyncNodeStatus(string, string, string, json.RawMessage) (int, string) + SyncHostNCVersion(string) GetPendingProgramIPConfigs() []IPConfigurationStatus GetAvailableIPConfigs() []IPConfigurationStatus GetAllocatedIPConfigs() []IPConfigurationStatus diff --git a/cns/configuration/configuration.go b/cns/configuration/configuration.go index 105fcc036a..b7a6ffdf56 100644 --- a/cns/configuration/configuration.go +++ b/cns/configuration/configuration.go @@ -17,14 +17,14 @@ const ( ) type CNSConfig struct { - TelemetrySettings TelemetrySettings - ManagedSettings ManagedSettings - ChannelMode string - UseHTTPS bool - TLSSubjectName string - TLSCertificatePath string - TLSPort string - WireserverIP string + TelemetrySettings TelemetrySettings + ManagedSettings ManagedSettings + ChannelMode string + UseHTTPS bool + TLSSubjectName string + TLSCertificatePath string + TLSPort string + SyncHostNCVersionIntervalSec int } type TelemetrySettings struct { @@ -128,4 +128,5 @@ func SetCNSConfigDefaults(config *CNSConfig) { if config.ChannelMode == "" { config.ChannelMode = cns.Direct } + config.SyncHostNCVersionIntervalSec = 30 } diff --git a/cns/fakes/cnsfake.go b/cns/fakes/cnsfake.go index 6cb5e8644f..52036d8454 100644 --- a/cns/fakes/cnsfake.go +++ b/cns/fakes/cnsfake.go @@ -230,6 +230,11 @@ func (fake *HTTPServiceFake) SyncNodeStatus(string, string, string, json.RawMess return 0, "" } +// SyncHostNCVersion will update HostVersion in containerstatus. +func (fake *HTTPServiceFake) SyncHostNCVersion(string) { + return +} + func (fake *HTTPServiceFake) GetPendingProgramIPConfigs() []cns.IPConfigurationStatus { ipconfigs := []cns.IPConfigurationStatus{} for _, ipconfig := range fake.IPStateManager.PendingProgramIPConfigState { diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 9d2a4ffb3d..71d22ccd9a 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -10,6 +10,7 @@ import ( "net/http" "net/http/httptest" "reflect" + "strconv" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/logger" @@ -143,6 +144,37 @@ func (service *HTTPRestService) SyncNodeStatus(dncEP, infraVnet, nodeID string, return } +// SyncHostNCVersion will check NC version from NMAgent and save it as host version in container status. +// If NMAgent updated, CNS will refresh the pending programming IP status. +func (service *HTTPRestService) SyncHostNCVersion(channelMode string) { + for i, containerstatus := range service.state.ContainerStatus { + // Will open a separate PR to convert all the NC version related variable to int. Change from string to int is a pain. + hostVersion, err := strconv.Atoi(containerstatus.HostVersion) + if err != nil { + log.Errorf("Received err when chagne containerstatus.HostVersion %s to int, err msg %v", containerstatus.HostVersion, err) + return + } + vmVersion, err := strconv.Atoi(containerstatus.VMVersion) + if err != nil { + log.Errorf("Received err when chagne containerstatus.VMVersion %s to int, err msg %v", containerstatus.VMVersion, err) + return + } + // host NC version is the NC version from NMAgent, if it's already keep up with NC version exist in VM, no update needed. + if hostVersion >= vmVersion { + continue + } else { + newHostNCVersion := service.imdsClient.GetNetworkContainerInfoFromHostWithoutToken() + service.Lock() + if channelMode == cns.KubernetesCRD { + service.UpdatePendingProgrammingIPs(strconv.Itoa(newHostNCVersion), containerstatus.CreateNetworkContainerRequest) + } + containerstatus.HostVersion = strconv.Itoa(newHostNCVersion) + service.state.ContainerStatus[i] = containerstatus + service.Unlock() + } + } +} + // This API will be called by CNS RequestController on CRD update. func (service *HTTPRestService) ReconcileNCState(ncRequest *cns.CreateNetworkContainerRequest, podInfoByIp map[string]cns.KubernetesPodInfo, scalar nnc.Scaler, spec nnc.NodeNetworkConfigSpec) int { // check if ncRequest is null, then return as there is no CRD state yet diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 843f0a98a7..3e56e4c840 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -125,6 +125,81 @@ func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) { } } +func TestSyncHostNCVersion(t *testing.T) { + // cns.KubernetesCRD has one more logic compared to other orchestrator type, so test both of them + orchestratorTypes := [6]string{cns.Kubernetes, cns.KubernetesCRD} + for _, orchestratorType := range orchestratorTypes { + testSyncHostNCVersion(t, orchestratorType) + } +} + +func testSyncHostNCVersion(t *testing.T, orchestratorType string) { + req := createNCReqeustForSyncHostNCVersion(t) + containerStatus := svc.state.ContainerStatus[req.NetworkContainerid] + if containerStatus.HostVersion != "-1" { + t.Errorf("Unexpected containerStatus.HostVersion %s, expeted host version should be -1 in string", containerStatus.HostVersion) + } + if containerStatus.VMVersion != "0" { + t.Errorf("Unexpected containerStatus.VMVersion %s, expeted VM version should be 0 in string", containerStatus.VMVersion) + } + // When sync host NC version, it will use the orchestratorType pass in. + svc.SyncHostNCVersion(orchestratorType) + containerStatus = svc.state.ContainerStatus[req.NetworkContainerid] + if containerStatus.HostVersion != "0" { + t.Errorf("Unexpected containerStatus.HostVersion %s, expeted host version should be 0 in string", containerStatus.HostVersion) + } + if containerStatus.VMVersion != "0" { + t.Errorf("Unexpected containerStatus.VMVersion %s, expeted VM version should be 0 in string", containerStatus.VMVersion) + } +} + +func TestPendingIPsGotUpdatedWhenSyncHostNCVersion(t *testing.T) { + req := createNCReqeustForSyncHostNCVersion(t) + containerStatus := svc.state.ContainerStatus[req.NetworkContainerid] + + receivedSecondaryIPConfigs := containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs + if len(receivedSecondaryIPConfigs) != 1 { + t.Errorf("Unexpected receivedSecondaryIPConfigs length %d, expeted length is 1", len(receivedSecondaryIPConfigs)) + } + for i, _ := range receivedSecondaryIPConfigs { + podIPConfigState := svc.PodIPConfigState[i] + if podIPConfigState.State != cns.PendingProgramming { + t.Errorf("Unexpected State %s, expeted State is %s, received %s, IP address is %s", podIPConfigState.State, cns.PendingProgramming, podIPConfigState.State, podIPConfigState.IPAddress) + } + } + svc.SyncHostNCVersion(cns.KubernetesCRD) + containerStatus = svc.state.ContainerStatus[req.NetworkContainerid] + + receivedSecondaryIPConfigs = containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs + if len(receivedSecondaryIPConfigs) != 1 { + t.Errorf("Unexpected receivedSecondaryIPConfigs length %d, expeted length is 1", len(receivedSecondaryIPConfigs)) + } + for i, _ := range receivedSecondaryIPConfigs { + podIPConfigState := svc.PodIPConfigState[i] + if podIPConfigState.State != cns.Available { + t.Errorf("Unexpected State %s, expeted State is %s, received %s, IP address is %s", podIPConfigState.State, cns.Available, podIPConfigState.State, podIPConfigState.IPAddress) + } + } +} + +func createNCReqeustForSyncHostNCVersion(t *testing.T) cns.CreateNetworkContainerRequest { + restartService() + setEnv(t) + setOrchestratorTypeInternal(cns.KubernetesCRD) + + // NC version set as 0 which is the default initial value. + ncVersion := 0 + secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig) + ncID := "testNc1" + + // Build secondaryIPConfig, it will have one item as {IPAddress:"10.0.0.16", NCVersion: 0} + ipAddress := "10.0.0.16" + secIPConfig := newSecondaryIPConfig(ipAddress, ncVersion) + ipID := uuid.New() + secondaryIPConfigs[ipID.String()] = secIPConfig + req := createNCReqInternal(t, secondaryIPConfigs, ncID, strconv.Itoa(ncVersion)) + return req +} func TestReconcileNCWithEmptyState(t *testing.T) { restartService() setEnv(t) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index cdcf20f5b9..d606b0a7be 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -8,6 +8,7 @@ import ( "encoding/json" "fmt" "net/http" + "strconv" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/logger" @@ -113,6 +114,24 @@ func (service *HTTPRestService) MarkIPsAsPending(numberToMark int) (map[string]c return nil, fmt.Errorf("Failed to mark %d IP's as pending, only marked %d IP's", numberToMark, len(pendingReleaseIPs)) } +// UpdatePendingProgrammingIPs will update pending programming IPs to available if +// NMAgent side's programmed NC version keep up with NC version with secondary IP. +// This function must be called in a service lock. +func (service *HTTPRestService) UpdatePendingProgrammingIPs(nmagentNCVersion string, req cns.CreateNetworkContainerRequest) { + for uuid, secondaryIPConfigs := range req.SecondaryIPConfigs { + ipConfigStatus, exist := service.PodIPConfigState[uuid] + if exist { + if ipConfigStatus.State == cns.PendingProgramming && strconv.Itoa(secondaryIPConfigs.NCVersion) <= nmagentNCVersion { + ipConfigStatus.State = cns.Available + service.PodIPConfigState[uuid] = ipConfigStatus + logger.Printf("Change ip %s with uuid %s from pending programming to %s", ipConfigStatus.IPAddress, uuid, cns.Available) + } + } else { + logger.Errorf("IP %s with uuid as %s exist in service state Secondary IP list but can't find in PodIPConfigState", ipConfigStatus.IPAddress, uuid) + } + } +} + func (service *HTTPRestService) GetPodIPConfigState() map[string]cns.IPConfigurationStatus { service.RLock() defer service.RUnlock() diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 1ca49b8de9..e09292cb70 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -243,9 +243,6 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN 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.GetNetworkContainerInfoFromHostWithoutToken() - if nmagentNCVersion >= newNCVersion { service.addIPConfigStateUntransacted(cns.Available, req.NetworkContainerid, newIPConfigs, existingSecondaryIPConfigs) } else { diff --git a/cns/service/main.go b/cns/service/main.go index f93978ebfa..a93ad79b18 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -633,6 +633,14 @@ func main() { } } + go func() { + // Periodically poll NC version from NMAgent + for { + <-time.NewTicker(time.Duration(cnsconfig.SyncHostNCVersionIntervalSec) * time.Second).C + httpRestService.SyncHostNCVersion(config.ChannelMode) + } + }() + // Relay these incoming signals to OS signal channel. osSignalChannel := make(chan os.Signal, 1) signal.Notify(osSignalChannel, os.Interrupt, os.Kill, syscall.SIGTERM) From d9c9b956e47587ad668e67dae3dd3644566221a9 Mon Sep 17 00:00:00 2001 From: Shufang Date: Thu, 29 Oct 2020 18:27:02 -0700 Subject: [PATCH 02/12] Update NC version in test from 0 to -1, which will allow default IP state as Avaialable instead of pending programming. --- cns/cnsclient/cnsclient_test.go | 3 +++ cns/restserver/internalapi_test.go | 8 ++++---- cns/restserver/ipam_test.go | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/cns/cnsclient/cnsclient_test.go b/cns/cnsclient/cnsclient_test.go index aeae417033..79b6fb6af4 100644 --- a/cns/cnsclient/cnsclient_test.go +++ b/cns/cnsclient/cnsclient_test.go @@ -63,6 +63,9 @@ func addTestStateToRestServer(t *testing.T, secondaryIps []string) { NetworkContainerid: "testNcId1", IPConfiguration: ipConfig, SecondaryIPConfigs: secondaryIPConfigs, + // Set it as -1 to be same as default host version. + // It will allow secondary IPs status to be set as available. + Version: "-1", } returnCode := svc.CreateOrUpdateNetworkContainerInternal(req, fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 3e56e4c840..61a5a16d6b 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -44,8 +44,8 @@ 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") + // NC version set as -1 which is the same as default host version value. + validateCreateOrUpdateNCInternal(t, 2, "-1") } func TestCreateOrUpdateNCWithLargerVersionComparedToNMAgent(t *testing.T) { @@ -230,7 +230,7 @@ func TestReconcileNCWithExistingState(t *testing.T) { secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ } - req := generateNetworkContainerRequest(secondaryIPConfigs, "reconcileNc1", "0") + req := generateNetworkContainerRequest(secondaryIPConfigs, "reconcileNc1", "-1") expectedAllocatedPods := make(map[string]cns.KubernetesPodInfo) expectedAllocatedPods["10.0.0.6"] = cns.KubernetesPodInfo{ @@ -267,7 +267,7 @@ func TestReconcileNCWithSystemPods(t *testing.T) { secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ } - req := generateNetworkContainerRequest(secondaryIPConfigs, uuid.New().String(), "0") + req := generateNetworkContainerRequest(secondaryIPConfigs, uuid.New().String(), "-1") expectedAllocatedPods := make(map[string]cns.KubernetesPodInfo) expectedAllocatedPods["10.0.0.6"] = cns.KubernetesPodInfo{ diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index c04c8fea2e..93680a629b 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -145,7 +145,7 @@ func UpdatePodIpConfigState(t *testing.T, svc *HTTPRestService, ipconfigs map[st secondaryIPConfigs[ipId] = secIpConfig } - createAndValidateNCRequest(t, secondaryIPConfigs, testNCID, "0") + createAndValidateNCRequest(t, secondaryIPConfigs, testNCID, "-1") // update ipconfigs to expected state for ipId, ipconfig := range ipconfigs { From 0d2e3cd8378dc23ca78c37cc6a4f286640ee6228 Mon Sep 17 00:00:00 2001 From: Shufang Date: Thu, 29 Oct 2020 22:54:30 -0700 Subject: [PATCH 03/12] Add secondary IP status updation when reconcile. Resovle conflicts manually. Update unit test nc version value. Update unit test nc version. Add get nmagent default value back for integ testing purpose. Unit test can be break by this change. Update default new IP CNS status to available. Assign value to host version if none exist in util.go Addressed feedback and perform cluster integ test with 1 sec frequent nc version update. Need to clean logNCSnapshots when send out PR. Update nc version associate with secondary ip. Add new nmagent api test. Add versionResponseWithoutToken.Containers log Add containerId from our runner sub. Add containerId from NMAgent team. Addressed feedback and add real nmagent logic. Add timeout when query nmagent for nc version. --- cns/api.go | 1 - cns/cnsclient/cnsclient_test.go | 3 +- cns/configuration/configuration.go | 24 +++--- cns/fakes/cnsfake.go | 2 +- cns/fakes/nmagentclientfake.go | 22 ++++++ cns/nmagentclient/nmagentclient.go | 78 ++++++++++++++++++- .../kubecontroller/crdtranslator.go | 4 + cns/restserver/api.go | 4 +- cns/restserver/api_test.go | 2 +- cns/restserver/const.go | 1 + cns/restserver/internalapi.go | 61 +++++++++++---- cns/restserver/internalapi_test.go | 54 ++++++++----- cns/restserver/ipam.go | 47 +++++++---- cns/restserver/ipam_test.go | 3 +- cns/restserver/restserver.go | 9 ++- cns/restserver/util.go | 45 ++++++----- cns/service/main.go | 24 +++--- 17 files changed, 287 insertions(+), 97 deletions(-) create mode 100644 cns/fakes/nmagentclientfake.go diff --git a/cns/api.go b/cns/api.go index 833fef4da4..61d8ec82e3 100644 --- a/cns/api.go +++ b/cns/api.go @@ -38,7 +38,6 @@ type HTTPService interface { SendNCSnapShotPeriodically(int, chan bool) SetNodeOrchestrator(*SetOrchestratorTypeRequest) SyncNodeStatus(string, string, string, json.RawMessage) (int, string) - SyncHostNCVersion(string) GetPendingProgramIPConfigs() []IPConfigurationStatus GetAvailableIPConfigs() []IPConfigurationStatus GetAllocatedIPConfigs() []IPConfigurationStatus diff --git a/cns/cnsclient/cnsclient_test.go b/cns/cnsclient/cnsclient_test.go index 79b6fb6af4..850438e922 100644 --- a/cns/cnsclient/cnsclient_test.go +++ b/cns/cnsclient/cnsclient_test.go @@ -53,6 +53,7 @@ func addTestStateToRestServer(t *testing.T, secondaryIps []string) { for _, secIpAddress := range secondaryIps { secIpConfig := cns.SecondaryIPConfig{ IPAddress: secIpAddress, + NCVersion: -1, } ipId := uuid.New() secondaryIPConfigs[ipId.String()] = secIpConfig @@ -125,7 +126,7 @@ func TestMain(m *testing.M) { logger.InitLogger(logName, 0, 0, tmpLogDir+"/") config := common.ServiceConfig{} - httpRestService, err := restserver.NewHTTPRestService(&config, fakes.NewFakeImdsClient()) + httpRestService, err := restserver.NewHTTPRestService(&config, fakes.NewFakeImdsClient(), fakes.NewFakeNMAgentClient()) svc = httpRestService.(*restserver.HTTPRestService) svc.Name = "cns-test-server" svc.IPAMPoolMonitor = fakes.NewIPAMPoolMonitorFake() diff --git a/cns/configuration/configuration.go b/cns/configuration/configuration.go index b7a6ffdf56..66e5ee5191 100644 --- a/cns/configuration/configuration.go +++ b/cns/configuration/configuration.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "os" "path/filepath" + "time" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/logger" @@ -17,14 +18,18 @@ const ( ) type CNSConfig struct { - TelemetrySettings TelemetrySettings - ManagedSettings ManagedSettings - ChannelMode string - UseHTTPS bool - TLSSubjectName string - TLSCertificatePath string - TLSPort string - SyncHostNCVersionIntervalSec int + TelemetrySettings TelemetrySettings + ManagedSettings ManagedSettings + ChannelMode string + UseHTTPS bool + TLSSubjectName string + TLSCertificatePath string + TLSPort string + TLSEndpoint string + WireserverIP string + SyncHostNCVersionIntervalSec int + SyncHostNCVersionIntervalMilliSec time.Duration + SyncHostNCTimeoutMilliSec time.Duration } type TelemetrySettings struct { @@ -128,5 +133,6 @@ func SetCNSConfigDefaults(config *CNSConfig) { if config.ChannelMode == "" { config.ChannelMode = cns.Direct } - config.SyncHostNCVersionIntervalSec = 30 + config.SyncHostNCVersionIntervalMilliSec = 1000 + config.SyncHostNCTimeoutMilliSec = 500 } diff --git a/cns/fakes/cnsfake.go b/cns/fakes/cnsfake.go index 52036d8454..7748749b87 100644 --- a/cns/fakes/cnsfake.go +++ b/cns/fakes/cnsfake.go @@ -230,7 +230,7 @@ func (fake *HTTPServiceFake) SyncNodeStatus(string, string, string, json.RawMess return 0, "" } -// SyncHostNCVersion will update HostVersion in containerstatus. +// SyncHostNCVersion will update HostNCVersion in containerstatus. func (fake *HTTPServiceFake) SyncHostNCVersion(string) { return } diff --git a/cns/fakes/nmagentclientfake.go b/cns/fakes/nmagentclientfake.go new file mode 100644 index 0000000000..e61a104dc2 --- /dev/null +++ b/cns/fakes/nmagentclientfake.go @@ -0,0 +1,22 @@ +// Copyright 2017 Microsoft. All rights reserved. +// MIT License + +package fakes + +// NMAgentClientTest can be used to query to VM Host info. +type NMAgentClientTest struct { +} + +// NewFakeNMAgentClient return a mock implemetation of NMAgentClient +func NewFakeNMAgentClient() *NMAgentClientTest { + return &NMAgentClientTest{} +} + +// GetNcVersionListWithOutToken is mock implementation to return nc version list. +func (nmagentclient *NMAgentClientTest) GetNcVersionListWithOutToken(ncNeedUpdateList []string) map[string]int { + ncVersionList := make(map[string]int) + for _, ncID := range ncNeedUpdateList { + ncVersionList[ncID] = 0 + } + return ncVersionList +} diff --git a/cns/nmagentclient/nmagentclient.go b/cns/nmagentclient/nmagentclient.go index e254a09547..930845b3fc 100644 --- a/cns/nmagentclient/nmagentclient.go +++ b/cns/nmagentclient/nmagentclient.go @@ -5,7 +5,10 @@ import ( "encoding/json" "encoding/xml" "fmt" + "io/ioutil" "net/http" + "strconv" + "time" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/common" @@ -13,12 +16,14 @@ import ( const ( //GetNmAgentSupportedApiURLFmt Api endpoint to get supported Apis of NMAgent - GetNmAgentSupportedApiURLFmt = "http://%s/machine/plugins/?comp=nmagent&type=GetSupportedApis" - GetNetworkContainerVersionURLFmt = "http://%s/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/%s/networkContainers/%s/version/authenticationToken/%s/api-version/1" + GetNmAgentSupportedApiURLFmt = "http://%s/machine/plugins/?comp=nmagent&type=GetSupportedApis" + GetNetworkContainerVersionURLFmt = "http://%s/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/%s/networkContainers/%s/version/authenticationToken/%s/api-version/1" + GetNcVersionListWithOutTokenURLFmt = "http://%s/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/api-version/%s" ) //WireServerIP - wire server ip var WireserverIP = "168.63.129.16" +var getNcVersionListWithOutTokenURLVersion = "2" // NMANetworkContainerResponse - NMAgent response. type NMANetworkContainerResponse struct { @@ -31,6 +36,36 @@ type NMAgentSupportedApisResponseXML struct { SupportedApis []string `xml:"type"` } +type ContainerInfo struct { + NetworkContainerID string `json:"networkContainerId"` + Version string `json:"version"` +} + +type NMANetworkContainerListResponse struct { + ResponseCode string `json:"httpStatusCode"` + Containers []ContainerInfo `json:"networkContainers"` +} + +// NMAgentClient is client to handle queries to nmagent +type NMAgentClient struct { + connectionURL string +} + +// NMAgentClientInterface has interface that nmagent client will handle +type NMAgentClientInterface interface { + GetNcVersionListWithOutToken(ncNeedUpdateList []string) map[string]int +} + +// NewNMAgentClient create a new nmagent client. +func NewNMAgentClient(url string) (*NMAgentClient, error) { + if url == "" { + url = fmt.Sprintf(GetNcVersionListWithOutTokenURLFmt, WireserverIP, getNcVersionListWithOutTokenURLVersion) + } + return &NMAgentClient{ + connectionURL: url, + }, nil +} + // JoinNetwork joins the given network func JoinNetwork( networkID string, @@ -149,3 +184,42 @@ func GetNmAgentSupportedApis( logger.Printf("[NMAgentClient][Response] GetNmAgentSupportedApis. Response: %+v.", response) return xmlDoc.SupportedApis, nil } + +// GetNcVersionListWithOutToken query nmagent for programmed container version. +func (nmagentclient *NMAgentClient) GetNcVersionListWithOutToken(ncNeedUpdateList []string) map[string]int { + ncVersionList := make(map[string]int) + now := time.Now() + response, err := http.Get(nmagentclient.connectionURL) + latency := time.Since(now) + logger.Printf("[NMAgentClient][Response] GetNcVersionListWithOutToken response: %+v. QueryURL is %s, latency is %v", response, nmagentclient.connectionURL, latency) + + if response.StatusCode != http.StatusOK { + logger.Printf("[NMAgentClient][Response] GetNcVersionListWithOutToken failed with %d, err is %v", response.StatusCode, err) + return nil + } + + var nmaNcListResponse NMANetworkContainerListResponse + rBytes, _ := ioutil.ReadAll(response.Body) + logger.Printf("Response body is %v", rBytes) + json.Unmarshal(rBytes, &nmaNcListResponse) + if nmaNcListResponse.ResponseCode != "200" { + logger.Printf("[NMAgentClient][Response] GetNcVersionListWithOutToken unmarshal failed with %s", rBytes) + return nil + } + + var receivedNcVersionListInMap = make(map[string]string) + for _, containers := range nmaNcListResponse.Containers { + receivedNcVersionListInMap[containers.NetworkContainerID] = containers.Version + } + for _, ncID := range ncNeedUpdateList { + if val, ok := receivedNcVersionListInMap[ncID]; ok { + if valInInt, err := strconv.Atoi(val); err != nil { + logger.Printf("[NMAgentClient][Response] GetNcVersionListWithOutToken translate version %s to int failed with %s", val, err) + } else { + ncVersionList[ncID] = valInInt + logger.Printf("Containers id is %s, version is %d", ncID, valInInt) + } + } + } + return ncVersionList +} diff --git a/cns/requestcontroller/kubecontroller/crdtranslator.go b/cns/requestcontroller/kubecontroller/crdtranslator.go index 4a57a68030..fdca0020e0 100644 --- a/cns/requestcontroller/kubecontroller/crdtranslator.go +++ b/cns/requestcontroller/kubecontroller/crdtranslator.go @@ -6,6 +6,7 @@ import ( "strconv" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/log" nnc "github.com/Azure/azure-container-networking/nodenetworkconfig/api/v1alpha" ) @@ -38,6 +39,8 @@ func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetw ncRequest.NetworkContainerid = nc.ID ncRequest.NetworkContainerType = cns.Docker ncRequest.Version = strconv.FormatInt(nc.Version, 10) + log.Printf("Set nc request info with SecondaryIPConfigs %v, NetworkContainerid %s, NetworkContainerType %s, NC Version %s", + ncRequest.SecondaryIPConfigs, ncRequest.NetworkContainerid, ncRequest.NetworkContainerType, ncRequest.Version) if ip = net.ParseIP(nc.PrimaryIP); ip == nil { return ncRequest, fmt.Errorf("Invalid PrimaryIP %s:", nc.PrimaryIP) @@ -66,6 +69,7 @@ func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetw NCVersion: ncVersion, } ncRequest.SecondaryIPConfigs[ipAssignment.Name] = secondaryIPConfig + log.Debugf("Seconday IP Configs got set, name is %s, config is %v", ipAssignment.Name, secondaryIPConfig) } } diff --git a/cns/restserver/api.go b/cns/restserver/api.go index bbcd719f4b..f5412caec3 100644 --- a/cns/restserver/api.go +++ b/cns/restserver/api.go @@ -786,7 +786,7 @@ func (service *HTTPRestService) createOrUpdateNetworkContainer(w http.ResponseWr existing, ok := service.getNetworkContainerDetails(req.NetworkContainerid) // create/update nc only if it doesn't exist or it exists and the requested version is different from the saved version - if !ok || (ok && existing.VMVersion != req.Version) { + if !ok || (ok && existing.DncNCVersion != req.Version) { nc := service.networkContainer if err = nc.Create(req); err != nil { returnMessage = fmt.Sprintf("[Azure CNS] Error. CreateOrUpdateNetworkContainer failed %v", err.Error()) @@ -799,7 +799,7 @@ func (service *HTTPRestService) createOrUpdateNetworkContainer(w http.ResponseWr existing, ok := service.getNetworkContainerDetails(req.NetworkContainerid) // create/update nc only if it doesn't exist or it exists and the requested version is different from the saved version - if ok && existing.VMVersion != req.Version { + if ok && existing.DncNCVersion != req.Version { nc := service.networkContainer netPluginConfig := service.getNetPluginDetails() if err = nc.Update(req, netPluginConfig); err != nil { diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 07551c71bc..e266a9b457 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -912,7 +912,7 @@ func startService() { var err error // Create the service. config := common.ServiceConfig{} - service, err = NewHTTPRestService(&config, fakes.NewFakeImdsClient()) + service, err = NewHTTPRestService(&config, fakes.NewFakeImdsClient(), fakes.NewFakeNMAgentClient()) if err != nil { fmt.Printf("Failed to create CNS object %v\n", err) os.Exit(1) diff --git a/cns/restserver/const.go b/cns/restserver/const.go index 8cb329cf05..bc868e8e4a 100644 --- a/cns/restserver/const.go +++ b/cns/restserver/const.go @@ -38,6 +38,7 @@ const ( NetworkContainerVfpProgramComplete = 35 NetworkContainerVfpProgramCheckSkipped = 36 NmAgentSupportedApisError = 37 + UnsupportedNCVersion = 37 UnexpectedError = 99 ) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 71d22ccd9a..fd7d2d231b 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -11,6 +11,7 @@ import ( "net/http/httptest" "reflect" "strconv" + "time" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/logger" @@ -144,33 +145,59 @@ func (service *HTTPRestService) SyncNodeStatus(dncEP, infraVnet, nodeID string, return } -// SyncHostNCVersion will check NC version from NMAgent and save it as host version in container status. -// If NMAgent updated, CNS will refresh the pending programming IP status. -func (service *HTTPRestService) SyncHostNCVersion(channelMode string) { - for i, containerstatus := range service.state.ContainerStatus { +// SyncHostNCVersion will check NC version from NMAgent and save it as host NC version in container status. +// If NMAgent NC version got updated, CNS will refresh the pending programming IP status. +func (service *HTTPRestService) SyncHostNCVersion(channelMode string, syncHostNCTimeoutMilliSec time.Duration) { + var hostVersionNeedUpdateNcList []string + service.RLock() + for _, containerstatus := range service.state.ContainerStatus { // Will open a separate PR to convert all the NC version related variable to int. Change from string to int is a pain. - hostVersion, err := strconv.Atoi(containerstatus.HostVersion) + hostNcVersion, err := strconv.Atoi(containerstatus.HostNCVersion) if err != nil { - log.Errorf("Received err when chagne containerstatus.HostVersion %s to int, err msg %v", containerstatus.HostVersion, err) + log.Errorf("Received err when change containerstatus.HostNCVersion %s to int, err msg %v", containerstatus.HostNCVersion, err) return } - vmVersion, err := strconv.Atoi(containerstatus.VMVersion) + dncNcVersion, err := strconv.Atoi(containerstatus.DncNCVersion) if err != nil { - log.Errorf("Received err when chagne containerstatus.VMVersion %s to int, err msg %v", containerstatus.VMVersion, err) + log.Errorf("Received err when change containerstatus.DncNCVersion %s to int, err msg %v", containerstatus.DncNCVersion, err) return } - // host NC version is the NC version from NMAgent, if it's already keep up with NC version exist in VM, no update needed. - if hostVersion >= vmVersion { - continue - } else { - newHostNCVersion := service.imdsClient.GetNetworkContainerInfoFromHostWithoutToken() + // host NC version is the NC version from NMAgent, if it's smaller than + if hostNcVersion < dncNcVersion { + hostVersionNeedUpdateNcList = append(hostVersionNeedUpdateNcList, containerstatus.ID) + } + } + service.RUnlock() + if len(hostVersionNeedUpdateNcList) > 0 { + c1 := make(chan map[string]int, 1) + go func() { + c1 <- service.nmagentClient.GetNcVersionListWithOutToken(hostVersionNeedUpdateNcList) + }() + select { + case newHostNCVersionList := <-c1: service.Lock() - if channelMode == cns.KubernetesCRD { - service.UpdatePendingProgrammingIPs(strconv.Itoa(newHostNCVersion), containerstatus.CreateNetworkContainerRequest) + if newHostNCVersionList == nil { + logger.Errorf("Can't get vfp programmed NC version list from url without token") + if channelMode == cns.CRD { + service.MarkAllPendingProgrammingIpsAsAvailableUntransacted() + } + } else { + for ncID, newHostNCVersion := range newHostNCVersionList { + // Check whether it exist in service state and get the related nc info + if ncInfo, exist := service.state.ContainerStatus[ncID]; !exist { + logger.Errorf("Can't find NC with ID %s in service state, stop updating this host NC version", ncID) + } else { + if channelMode == cns.CRD { + service.MarkIpsAsAvailableUntransacted(ncInfo.ID, newHostNCVersion) + } + ncInfo.HostNCVersion = strconv.Itoa(newHostNCVersion) + service.state.ContainerStatus[ncID] = ncInfo + } + } } - containerstatus.HostVersion = strconv.Itoa(newHostNCVersion) - service.state.ContainerStatus[i] = containerstatus service.Unlock() + case <-time.After(syncHostNCTimeoutMilliSec * time.Millisecond): + logger.Errorf("Timeout when getting vfp programmed NC version list from url without token") } } } diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 61a5a16d6b..b743a6f853 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -54,7 +54,7 @@ func TestCreateOrUpdateNCWithLargerVersionComparedToNMAgent(t *testing.T) { setEnv(t) setOrchestratorTypeInternal(cns.KubernetesCRD) // NC version set as 1 which is larger than NC version get from mock nmagent. - validateCreateOrUpdateNCInternal(t, 2, "1") + validateCreateNCInternal(t, 2, "1") } func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) { @@ -127,7 +127,7 @@ func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) { func TestSyncHostNCVersion(t *testing.T) { // cns.KubernetesCRD has one more logic compared to other orchestrator type, so test both of them - orchestratorTypes := [6]string{cns.Kubernetes, cns.KubernetesCRD} + orchestratorTypes := []string{cns.Kubernetes, cns.KubernetesCRD} for _, orchestratorType := range orchestratorTypes { testSyncHostNCVersion(t, orchestratorType) } @@ -136,20 +136,20 @@ func TestSyncHostNCVersion(t *testing.T) { func testSyncHostNCVersion(t *testing.T, orchestratorType string) { req := createNCReqeustForSyncHostNCVersion(t) containerStatus := svc.state.ContainerStatus[req.NetworkContainerid] - if containerStatus.HostVersion != "-1" { - t.Errorf("Unexpected containerStatus.HostVersion %s, expeted host version should be -1 in string", containerStatus.HostVersion) + if containerStatus.HostNCVersion != "-1" { + t.Errorf("Unexpected containerStatus.HostNCVersion %s, expeted host version should be -1 in string", containerStatus.HostNCVersion) } - if containerStatus.VMVersion != "0" { - t.Errorf("Unexpected containerStatus.VMVersion %s, expeted VM version should be 0 in string", containerStatus.VMVersion) + if containerStatus.DncNCVersion != "0" { + t.Errorf("Unexpected containerStatus.DncNCVersion %s, expeted VM version should be 0 in string", containerStatus.DncNCVersion) } // When sync host NC version, it will use the orchestratorType pass in. - svc.SyncHostNCVersion(orchestratorType) + svc.SyncHostNCVersion(orchestratorType, 500) containerStatus = svc.state.ContainerStatus[req.NetworkContainerid] - if containerStatus.HostVersion != "0" { - t.Errorf("Unexpected containerStatus.HostVersion %s, expeted host version should be 0 in string", containerStatus.HostVersion) + if containerStatus.HostNCVersion != "0" { + t.Errorf("Unexpected containerStatus.HostNCVersion %s, expeted host version should be 0 in string", containerStatus.HostNCVersion) } - if containerStatus.VMVersion != "0" { - t.Errorf("Unexpected containerStatus.VMVersion %s, expeted VM version should be 0 in string", containerStatus.VMVersion) + if containerStatus.DncNCVersion != "0" { + t.Errorf("Unexpected containerStatus.DncNCVersion %s, expeted VM version should be 0 in string", containerStatus.DncNCVersion) } } @@ -167,7 +167,7 @@ func TestPendingIPsGotUpdatedWhenSyncHostNCVersion(t *testing.T) { t.Errorf("Unexpected State %s, expeted State is %s, received %s, IP address is %s", podIPConfigState.State, cns.PendingProgramming, podIPConfigState.State, podIPConfigState.IPAddress) } } - svc.SyncHostNCVersion(cns.KubernetesCRD) + svc.SyncHostNCVersion(cns.CRD, 500) containerStatus = svc.state.ContainerStatus[req.NetworkContainerid] receivedSecondaryIPConfigs = containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs @@ -225,7 +225,7 @@ func TestReconcileNCWithExistingState(t *testing.T) { var startingIndex = 6 for i := 0; i < 4; i++ { ipaddress := "10.0.0." + strconv.Itoa(startingIndex) - secIpConfig := newSecondaryIPConfig(ipaddress, 0) + secIpConfig := newSecondaryIPConfig(ipaddress, -1) ipId := uuid.New() secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ @@ -262,7 +262,7 @@ func TestReconcileNCWithSystemPods(t *testing.T) { var startingIndex = 6 for i := 0; i < 4; i++ { ipaddress := "10.0.0." + strconv.Itoa(startingIndex) - secIpConfig := newSecondaryIPConfig(ipaddress, 0) + secIpConfig := newSecondaryIPConfig(ipaddress, -1) ipId := uuid.New() secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ @@ -296,14 +296,30 @@ func setOrchestratorTypeInternal(orchestratorType string) { svc.state.OrchestratorType = orchestratorType } -func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int, ncVersion string) { +func validateCreateNCInternal(t *testing.T, secondaryIpCount int, ncVersion string) { secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig) ncId := "testNc1" + ncVersionInInt, _ := strconv.Atoi(ncVersion) + var startingIndex = 6 + for i := 0; i < secondaryIpCount; i++ { + ipaddress := "10.0.0." + strconv.Itoa(startingIndex) + secIpConfig := newSecondaryIPConfig(ipaddress, ncVersionInInt) + ipId := uuid.New() + secondaryIPConfigs[ipId.String()] = secIpConfig + startingIndex++ + } + + createAndValidateNCRequest(t, secondaryIPConfigs, ncId, ncVersion) +} +func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int, ncVersion string) { + secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig) + ncId := "testNc1" + ncVersionInInt, _ := strconv.Atoi(ncVersion) var startingIndex = 6 for i := 0; i < secondaryIpCount; i++ { ipaddress := "10.0.0." + strconv.Itoa(startingIndex) - secIpConfig := newSecondaryIPConfig(ipaddress, 0) + secIpConfig := newSecondaryIPConfig(ipaddress, ncVersionInInt) ipId := uuid.New() secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ @@ -315,7 +331,7 @@ func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int, ncVers fmt.Println("Validate Scaleup") for i := 0; i < secondaryIpCount; i++ { ipaddress := "10.0.0." + strconv.Itoa(startingIndex) - secIpConfig := newSecondaryIPConfig(ipaddress, 1) + secIpConfig := newSecondaryIPConfig(ipaddress, ncVersionInInt) ipId := uuid.New() secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ @@ -379,12 +395,12 @@ func validateNetworkRequest(t *testing.T, req cns.CreateNetworkContainerRequest) var expectedIPStatus string // 0 is the default NMAgent version return from fake GetNetworkContainerInfoFromHost - if containerStatus.VMVersion > "0" { + if containerStatus.DncNCVersion > "0" { expectedIPStatus = cns.PendingProgramming } else { expectedIPStatus = cns.Available } - t.Logf("VMVersion is %s, HostVersion is %s", containerStatus.VMVersion, containerStatus.HostVersion) + t.Logf("DncNCVersion is %s, HostNCVersion is %s", containerStatus.DncNCVersion, containerStatus.HostNCVersion) var alreadyValidated = make(map[string]string) for ipid, ipStatus := range svc.PodIPConfigState { if ipaddress, found := alreadyValidated[ipid]; !found { diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index d606b0a7be..c55241dcaa 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -114,20 +114,41 @@ func (service *HTTPRestService) MarkIPsAsPending(numberToMark int) (map[string]c return nil, fmt.Errorf("Failed to mark %d IP's as pending, only marked %d IP's", numberToMark, len(pendingReleaseIPs)) } -// UpdatePendingProgrammingIPs will update pending programming IPs to available if -// NMAgent side's programmed NC version keep up with NC version with secondary IP. -// This function must be called in a service lock. -func (service *HTTPRestService) UpdatePendingProgrammingIPs(nmagentNCVersion string, req cns.CreateNetworkContainerRequest) { - for uuid, secondaryIPConfigs := range req.SecondaryIPConfigs { - ipConfigStatus, exist := service.PodIPConfigState[uuid] - if exist { - if ipConfigStatus.State == cns.PendingProgramming && strconv.Itoa(secondaryIPConfigs.NCVersion) <= nmagentNCVersion { - ipConfigStatus.State = cns.Available - service.PodIPConfigState[uuid] = ipConfigStatus - logger.Printf("Change ip %s with uuid %s from pending programming to %s", ipConfigStatus.IPAddress, uuid, cns.Available) +// MarkAllPendingProgrammingIpsAsAvailableUntransacted is the function to update pending programming IPs to available +// when get NC version failed and we don't want to block IP allocation. +// Note: this func is an untransacted API as the caller will take a Service lock +func (service *HTTPRestService) MarkAllPendingProgrammingIpsAsAvailableUntransacted() { + for _, ipConfigStatus := range service.PodIPConfigState { + if ipConfigStatus.State == cns.PendingProgramming { + ipConfigStatus.State = cns.Available + service.PodIPConfigState[ipConfigStatus.ID] = ipConfigStatus + } + } +} + +// MarkIpsAsAvailableUntransacted will update pending programming IPs to available if NMAgent side's programmed nc version keep up with nc version. +// Note: this func is an untransacted API as the caller will take a Service lock +func (service *HTTPRestService) MarkIpsAsAvailableUntransacted(ncID string, newHostNCVersion int) { + // Check whether it exist in service state and get the related nc info + if ncInfo, exist := service.state.ContainerStatus[ncID]; !exist { + logger.Errorf("Can't find NC with ID %s in service state, stop updating its pending programming IP status", ncID) + } else { + previousHostNCVersion := ncInfo.HostNCVersion + // We only need to handle the situation when dnc nc version is larger than programmed nc version + if previousHostNCVersion < ncInfo.DncNCVersion && previousHostNCVersion < strconv.Itoa(newHostNCVersion) { + for uuid, secondaryIPConfigs := range ncInfo.CreateNetworkContainerRequest.SecondaryIPConfigs { + if ipConfigStatus, exist := service.PodIPConfigState[uuid]; !exist { + logger.Errorf("IP %s with uuid as %s exist in service state Secondary IP list but can't find in PodIPConfigState", ipConfigStatus.IPAddress, uuid) + } else if ipConfigStatus.State == cns.PendingProgramming && secondaryIPConfigs.NCVersion <= newHostNCVersion { + ipConfigStatus.State = cns.Available + service.PodIPConfigState[uuid] = ipConfigStatus + // Following 2 sentence assign new host version to secondary ip config. + secondaryIPConfigs.NCVersion = newHostNCVersion + ncInfo.CreateNetworkContainerRequest.SecondaryIPConfigs[uuid] = secondaryIPConfigs + logger.Printf("Change ip %s with uuid %s from pending programming to %s, current secondary ip configs is %v", ipConfigStatus.IPAddress, uuid, cns.Available, + ncInfo.CreateNetworkContainerRequest.SecondaryIPConfigs[uuid]) + } } - } else { - logger.Errorf("IP %s with uuid as %s exist in service state Secondary IP list but can't find in PodIPConfigState", ipConfigStatus.IPAddress, uuid) } } } diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 93680a629b..9b4cd89304 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -41,7 +41,7 @@ var ( func getTestService() *HTTPRestService { var config common.ServiceConfig - httpsvc, _ := NewHTTPRestService(&config, fakes.NewFakeImdsClient()) + httpsvc, _ := NewHTTPRestService(&config, fakes.NewFakeImdsClient(), fakes.NewFakeNMAgentClient()) svc = httpsvc.(*HTTPRestService) svc.IPAMPoolMonitor = fakes.NewIPAMPoolMonitorFake() setOrchestratorTypeInternal(cns.KubernetesCRD) @@ -139,6 +139,7 @@ func UpdatePodIpConfigState(t *testing.T, svc *HTTPRestService, ipconfigs map[st for _, ipconfig := range ipconfigs { secIpConfig := cns.SecondaryIPConfig{ IPAddress: ipconfig.IPAddress, + NCVersion: -1, } ipId := ipconfig.ID diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index 91010fc71a..bd9b1df468 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -14,6 +14,7 @@ import ( "github.com/Azure/azure-container-networking/cns/ipamclient" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/networkcontainers" + "github.com/Azure/azure-container-networking/cns/nmagentclient" "github.com/Azure/azure-container-networking/cns/routes" acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/store" @@ -38,6 +39,7 @@ type HTTPRestService struct { dockerClient *dockerclient.DockerClient imdsClient imdsclient.ImdsClientInterface ipamClient *ipamclient.IpamClient + nmagentClient nmagentclient.NMAgentClientInterface networkContainer *networkcontainers.NetworkContainers PodIPIDByOrchestratorContext map[string]string // OrchestratorContext is key and value is Pod IP uuid. PodIPConfigState map[string]cns.IPConfigurationStatus // seondaryipid(uuid) is key @@ -57,8 +59,8 @@ type allocatedIPCount struct { // containerstatus is used to save status of an existing container type containerstatus struct { ID string - VMVersion string - HostVersion string + DncNCVersion string + HostNCVersion string CreateNetworkContainerRequest cns.CreateNetworkContainerRequest VfpUpdateComplete bool // True when VFP programming is completed for the NC } @@ -84,7 +86,7 @@ type networkInfo struct { } // NewHTTPRestService creates a new HTTP Service object. -func NewHTTPRestService(config *common.ServiceConfig, imdsClientInterface imdsclient.ImdsClientInterface) (cns.HTTPService, error) { +func NewHTTPRestService(config *common.ServiceConfig, imdsClientInterface imdsclient.ImdsClientInterface, nmagentClient nmagentclient.NMAgentClientInterface) (cns.HTTPService, error) { service, err := cns.NewService(config.Name, config.Version, config.ChannelMode, config.Store) if err != nil { return nil, err @@ -118,6 +120,7 @@ func NewHTTPRestService(config *common.ServiceConfig, imdsClientInterface imdscl dockerClient: dc, imdsClient: imdsClient, ipamClient: ic, + nmagentClient: nmagentClient, networkContainer: nc, PodIPIDByOrchestratorContext: podIPIDByOrchestratorContext, PodIPConfigState: podIPConfigState, diff --git a/cns/restserver/util.go b/cns/restserver/util.go index e09292cb70..8e28e82faa 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -106,7 +106,7 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw defer service.Unlock() var ( - hostVersion string + hostNCVersion string existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig //uuid is key vfpUpdateComplete bool ) @@ -117,12 +117,14 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw existingNCStatus, ok := service.state.ContainerStatus[req.NetworkContainerid] if ok { - hostVersion = existingNCStatus.HostVersion + hostNCVersion = existingNCStatus.HostNCVersion existingSecondaryIPConfigs = existingNCStatus.CreateNetworkContainerRequest.SecondaryIPConfigs vfpUpdateComplete = existingNCStatus.VfpUpdateComplete - } else { + } + if hostNCVersion == "" { // Host version is the NC version from NMAgent, set it -1 to indicate no result from NMAgent yet. - hostVersion = "-1" + // TODO, query NMAgent and with aggresive time out and assign latest host version. + hostNCVersion = "-1" } // Remove the auth token before saving the containerStatus to cns json file @@ -132,9 +134,9 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw service.state.ContainerStatus[req.NetworkContainerid] = containerstatus{ ID: req.NetworkContainerid, - VMVersion: req.Version, CreateNetworkContainerRequest: createNetworkContainerRequest, - HostVersion: hostVersion, + DncNCVersion: req.Version, + HostNCVersion: hostNCVersion, VfpUpdateComplete: vfpUpdateComplete} switch req.NetworkContainerType { @@ -179,7 +181,7 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw case cns.KubernetesCRD: // Validate and Update the SecondaryIpConfig state - returnCode, returnMesage := service.updateIpConfigsStateUntransacted(req, existingSecondaryIPConfigs, hostVersion) + returnCode, returnMesage := service.updateIpConfigsStateUntransacted(req, existingSecondaryIPConfigs, hostNCVersion) if returnCode != 0 { return returnCode, returnMesage } @@ -199,9 +201,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 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, hostVersion string) (int, string) { +func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateNetworkContainerRequest, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig, hostNCVersion string) (int, string) { // parse the existingSecondaryIpConfigState to find the deleted Ips newIPConfigs := req.SecondaryIPConfigs var tobeDeletedIpConfigs = make(map[string]cns.SecondaryIPConfig) @@ -240,14 +242,14 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN } } - newNCVersion, _ := strconv.Atoi(req.Version) - nmagentNCVersion, _ := strconv.Atoi(hostVersion) - - if nmagentNCVersion >= newNCVersion { - service.addIPConfigStateUntransacted(cns.Available, req.NetworkContainerid, newIPConfigs, existingSecondaryIPConfigs) - } else { - service.addIPConfigStateUntransacted(cns.PendingProgramming, req.NetworkContainerid, newIPConfigs, existingSecondaryIPConfigs) + // Add new IPs + // TODO, will udpate NC version related variable to int, change it from string to int is a pains + var hostNCVersionInInt int + var err error + if hostNCVersionInInt, err = strconv.Atoi(hostNCVersion); err != nil { + return UnsupportedNCVersion, fmt.Sprintf("Invalid hostNCVersion is %s, err:%s", hostNCVersion, err) } + service.addIPConfigStateUntransacted(req.NetworkContainerid, hostNCVersionInInt, req.SecondaryIPConfigs, existingSecondaryIPConfigs) return 0, "" } @@ -255,7 +257,8 @@ 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(newIPCNSStatus, ncId string, ipconfigs, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig) { +func (service *HTTPRestService) addIPConfigStateUntransacted(ncId string, hostNCVersion int, ipconfigs, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig) { + newIPCNSStatus := cns.Available // add ipconfigs to state for ipId, ipconfig := range ipconfigs { // New secondary IP configs has new NC version however, CNS don't want to override existing IPs'with new NC version @@ -263,11 +266,17 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(newIPCNSStatus, ncI if existingIPConfig, existsInPreviousIPConfig := existingSecondaryIPConfigs[ipId]; existsInPreviousIPConfig { ipconfig.NCVersion = existingIPConfig.NCVersion ipconfigs[ipId] = ipconfig + hostNCVersion = existingIPConfig.NCVersion } - logger.Printf("[Azure-Cns] Set IP %s version to %d", ipconfig.IPAddress, ipconfig.NCVersion) + logger.Printf("[Azure-Cns] Set IP %s version to %d, programmed host nc version is %d", ipconfig.IPAddress, ipconfig.NCVersion, hostNCVersion) if _, exists := service.PodIPConfigState[ipId]; exists { continue } + // Using the updated NC version attached with IP to compare with latest nmagent version and determine IP statues. + // When reconcile, service.PodIPConfigState doens't exist, rebuild it with the help of NC version attached with IP. + if hostNCVersion < ipconfig.NCVersion { + newIPCNSStatus = cns.PendingProgramming + } // add the new State ipconfigStatus := cns.IPConfigurationStatus{ NCID: ncId, diff --git a/cns/service/main.go b/cns/service/main.go index a93ad79b18..ad4fd701aa 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -453,8 +453,13 @@ func main() { return } + nmaclient, err := nmagentclient.NewNMAgentClient("") + if err != nil { + logger.Errorf("Failed to start nmagent client due to error %v\n", err) + return + } // Create CNS object. - httpRestService, err := restserver.NewHTTPRestService(&config, new(imdsclient.ImdsClient)) + httpRestService, err := restserver.NewHTTPRestService(&config, new(imdsclient.ImdsClient), nmaclient) if err != nil { logger.Errorf("Failed to create CNS object, err:%v.\n", err) return @@ -562,6 +567,15 @@ func main() { return } + logger.Printf("Starting SyncHostNCVersion") + go func() { + // Periodically poll vfp programmed NC version from NMAgent + for { + <-time.NewTicker(cnsconfig.SyncHostNCVersionIntervalMilliSec * time.Second).C + httpRestServiceImplementation.SyncHostNCVersion(config.ChannelMode, cnsconfig.SyncHostNCTimeoutMilliSec) + } + }() + // initialize the ipam pool monitor httpRestServiceImplementation.IPAMPoolMonitor = ipampoolmonitor.NewCNSIPAMPoolMonitor(httpRestServiceImplementation, requestController) @@ -633,14 +647,6 @@ func main() { } } - go func() { - // Periodically poll NC version from NMAgent - for { - <-time.NewTicker(time.Duration(cnsconfig.SyncHostNCVersionIntervalSec) * time.Second).C - httpRestService.SyncHostNCVersion(config.ChannelMode) - } - }() - // Relay these incoming signals to OS signal channel. osSignalChannel := make(chan os.Signal, 1) signal.Notify(osSignalChannel, os.Interrupt, os.Kill, syscall.SIGTERM) From 19afda89158555e5ee7da770bcf35969c9bd6bf1 Mon Sep 17 00:00:00 2001 From: Shufang Date: Tue, 17 Nov 2020 21:08:49 -0800 Subject: [PATCH 04/12] Update comments. --- cns/configuration/configuration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/configuration/configuration.go b/cns/configuration/configuration.go index 66e5ee5191..866c802920 100644 --- a/cns/configuration/configuration.go +++ b/cns/configuration/configuration.go @@ -126,7 +126,7 @@ func setManagedSettingDefaults(managedSettings *ManagedSettings) { } } -// Set Default values of CNS config if not specified +// SetCNSConfigDefaults set default values of CNS config if not specified func SetCNSConfigDefaults(config *CNSConfig) { setTelemetrySettingDefaults(&config.TelemetrySettings) setManagedSettingDefaults(&config.ManagedSettings) From 8dc5632586681a0c45f3164ab73c3c51efb59f6f Mon Sep 17 00:00:00 2001 From: Shufang Date: Wed, 18 Nov 2020 00:03:16 -0800 Subject: [PATCH 05/12] Add context background with timeout function for syncing node nc version. --- cns/restserver/internalapi.go | 6 +++--- cns/restserver/internalapi_test.go | 5 +++-- cns/service/main.go | 4 +++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index fd7d2d231b..7eea40e9dc 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -5,13 +5,13 @@ package restserver import ( "bytes" + "context" "encoding/json" "fmt" "net/http" "net/http/httptest" "reflect" "strconv" - "time" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/logger" @@ -147,7 +147,7 @@ func (service *HTTPRestService) SyncNodeStatus(dncEP, infraVnet, nodeID string, // SyncHostNCVersion will check NC version from NMAgent and save it as host NC version in container status. // If NMAgent NC version got updated, CNS will refresh the pending programming IP status. -func (service *HTTPRestService) SyncHostNCVersion(channelMode string, syncHostNCTimeoutMilliSec time.Duration) { +func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMode string) { var hostVersionNeedUpdateNcList []string service.RLock() for _, containerstatus := range service.state.ContainerStatus { @@ -196,7 +196,7 @@ func (service *HTTPRestService) SyncHostNCVersion(channelMode string, syncHostNC } } service.Unlock() - case <-time.After(syncHostNCTimeoutMilliSec * time.Millisecond): + case <-ctx.Done(): logger.Errorf("Timeout when getting vfp programmed NC version list from url without token") } } diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index b743a6f853..11af26eaff 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -4,6 +4,7 @@ package restserver import ( + "context" "encoding/json" "fmt" "reflect" @@ -143,7 +144,7 @@ func testSyncHostNCVersion(t *testing.T, orchestratorType string) { t.Errorf("Unexpected containerStatus.DncNCVersion %s, expeted VM version should be 0 in string", containerStatus.DncNCVersion) } // When sync host NC version, it will use the orchestratorType pass in. - svc.SyncHostNCVersion(orchestratorType, 500) + svc.SyncHostNCVersion(context.Background(), orchestratorType) containerStatus = svc.state.ContainerStatus[req.NetworkContainerid] if containerStatus.HostNCVersion != "0" { t.Errorf("Unexpected containerStatus.HostNCVersion %s, expeted host version should be 0 in string", containerStatus.HostNCVersion) @@ -167,7 +168,7 @@ func TestPendingIPsGotUpdatedWhenSyncHostNCVersion(t *testing.T) { t.Errorf("Unexpected State %s, expeted State is %s, received %s, IP address is %s", podIPConfigState.State, cns.PendingProgramming, podIPConfigState.State, podIPConfigState.IPAddress) } } - svc.SyncHostNCVersion(cns.CRD, 500) + svc.SyncHostNCVersion(context.Background(), cns.CRD) containerStatus = svc.state.ContainerStatus[req.NetworkContainerid] receivedSecondaryIPConfigs = containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs diff --git a/cns/service/main.go b/cns/service/main.go index ad4fd701aa..8c52c2014b 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -568,11 +568,13 @@ func main() { } logger.Printf("Starting SyncHostNCVersion") + rootCxt := context.Background() + ctxWithTimeout, _ := context.WithTimeout(rootCxt, cnsconfig.SyncHostNCTimeoutMilliSec) go func() { // Periodically poll vfp programmed NC version from NMAgent for { <-time.NewTicker(cnsconfig.SyncHostNCVersionIntervalMilliSec * time.Second).C - httpRestServiceImplementation.SyncHostNCVersion(config.ChannelMode, cnsconfig.SyncHostNCTimeoutMilliSec) + httpRestServiceImplementation.SyncHostNCVersion(ctxWithTimeout, config.ChannelMode) } }() From 3e32105abfe34dfbb8656286e7ff86890314a2e7 Mon Sep 17 00:00:00 2001 From: Shufang Date: Mon, 23 Nov 2020 23:53:04 -0800 Subject: [PATCH 06/12] Add 5 second force update CNS pending programming IP to available logic. --- cns/configuration/configuration.go | 2 ++ cns/fakes/cnsfake.go | 4 +++- cns/restserver/internalapi.go | 10 ++++++++-- cns/restserver/internalapi_test.go | 5 +++-- cns/service/main.go | 3 ++- 5 files changed, 18 insertions(+), 6 deletions(-) diff --git a/cns/configuration/configuration.go b/cns/configuration/configuration.go index 866c802920..a824d1231f 100644 --- a/cns/configuration/configuration.go +++ b/cns/configuration/configuration.go @@ -30,6 +30,7 @@ type CNSConfig struct { SyncHostNCVersionIntervalSec int SyncHostNCVersionIntervalMilliSec time.Duration SyncHostNCTimeoutMilliSec time.Duration + ForceMarkIPAvailableTimeRange time.Duration } type TelemetrySettings struct { @@ -135,4 +136,5 @@ func SetCNSConfigDefaults(config *CNSConfig) { } config.SyncHostNCVersionIntervalMilliSec = 1000 config.SyncHostNCTimeoutMilliSec = 500 + config.ForceMarkIPAvailableTimeRange = 5000 } diff --git a/cns/fakes/cnsfake.go b/cns/fakes/cnsfake.go index 7748749b87..1d62b11f80 100644 --- a/cns/fakes/cnsfake.go +++ b/cns/fakes/cnsfake.go @@ -1,9 +1,11 @@ package fakes import ( + "context" "encoding/json" "errors" "sync" + "time" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" @@ -231,7 +233,7 @@ func (fake *HTTPServiceFake) SyncNodeStatus(string, string, string, json.RawMess } // SyncHostNCVersion will update HostNCVersion in containerstatus. -func (fake *HTTPServiceFake) SyncHostNCVersion(string) { +func (fake *HTTPServiceFake) SyncHostNCVersion(context.Context, string, time.Time, time.Duration) { return } diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 7eea40e9dc..8318ff74a3 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -12,6 +12,7 @@ import ( "net/http/httptest" "reflect" "strconv" + "time" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/logger" @@ -147,7 +148,7 @@ func (service *HTTPRestService) SyncNodeStatus(dncEP, infraVnet, nodeID string, // SyncHostNCVersion will check NC version from NMAgent and save it as host NC version in container status. // If NMAgent NC version got updated, CNS will refresh the pending programming IP status. -func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMode string) { +func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMode string, lastUpdatedHostNCTimeStamp time.Time, forceMarkIPAvailableTimeRange time.Duration) { var hostVersionNeedUpdateNcList []string service.RLock() for _, containerstatus := range service.state.ContainerStatus { @@ -178,8 +179,10 @@ func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMo service.Lock() if newHostNCVersionList == nil { logger.Errorf("Can't get vfp programmed NC version list from url without token") - if channelMode == cns.CRD { + if channelMode == cns.CRD && time.Since(lastUpdatedHostNCTimeStamp) > forceMarkIPAvailableTimeRange { service.MarkAllPendingProgrammingIpsAsAvailableUntransacted() + lastUpdatedHostNCTimeStamp = time.Now() + logger.Printf("Can't get vfp programmed NC version list from url without token and performed force update.") } } else { for ncID, newHostNCVersion := range newHostNCVersionList { @@ -194,11 +197,14 @@ func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMo service.state.ContainerStatus[ncID] = ncInfo } } + lastUpdatedHostNCTimeStamp = time.Now() } service.Unlock() case <-ctx.Done(): logger.Errorf("Timeout when getting vfp programmed NC version list from url without token") } + } else { + lastUpdatedHostNCTimeStamp = time.Now() } } diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 11af26eaff..4a441b4d6a 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -10,6 +10,7 @@ import ( "reflect" "strconv" "testing" + "time" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/fakes" @@ -144,7 +145,7 @@ func testSyncHostNCVersion(t *testing.T, orchestratorType string) { t.Errorf("Unexpected containerStatus.DncNCVersion %s, expeted VM version should be 0 in string", containerStatus.DncNCVersion) } // When sync host NC version, it will use the orchestratorType pass in. - svc.SyncHostNCVersion(context.Background(), orchestratorType) + svc.SyncHostNCVersion(context.Background(), orchestratorType, time.Now(), 5*time.Second) containerStatus = svc.state.ContainerStatus[req.NetworkContainerid] if containerStatus.HostNCVersion != "0" { t.Errorf("Unexpected containerStatus.HostNCVersion %s, expeted host version should be 0 in string", containerStatus.HostNCVersion) @@ -168,7 +169,7 @@ func TestPendingIPsGotUpdatedWhenSyncHostNCVersion(t *testing.T) { t.Errorf("Unexpected State %s, expeted State is %s, received %s, IP address is %s", podIPConfigState.State, cns.PendingProgramming, podIPConfigState.State, podIPConfigState.IPAddress) } } - svc.SyncHostNCVersion(context.Background(), cns.CRD) + svc.SyncHostNCVersion(context.Background(), cns.CRD, time.Now(), 5*time.Second) containerStatus = svc.state.ContainerStatus[req.NetworkContainerid] receivedSecondaryIPConfigs = containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs diff --git a/cns/service/main.go b/cns/service/main.go index 8c52c2014b..64454a267d 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -568,13 +568,14 @@ func main() { } logger.Printf("Starting SyncHostNCVersion") + lastUpdatedHostNCTimeStamp := time.Now() rootCxt := context.Background() ctxWithTimeout, _ := context.WithTimeout(rootCxt, cnsconfig.SyncHostNCTimeoutMilliSec) go func() { // Periodically poll vfp programmed NC version from NMAgent for { <-time.NewTicker(cnsconfig.SyncHostNCVersionIntervalMilliSec * time.Second).C - httpRestServiceImplementation.SyncHostNCVersion(ctxWithTimeout, config.ChannelMode) + httpRestServiceImplementation.SyncHostNCVersion(ctxWithTimeout, config.ChannelMode, lastUpdatedHostNCTimeStamp, cnsconfig.ForceMarkIPAvailableTimeRange) } }() From 88748fecdb2e3bacb9478e54d4dea11fcc42fd56 Mon Sep 17 00:00:00 2001 From: Shufang Date: Tue, 24 Nov 2020 11:03:00 -0800 Subject: [PATCH 07/12] Resovle merge conflict from master. --- cns/nmagentclient/nmagentclient.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cns/nmagentclient/nmagentclient.go b/cns/nmagentclient/nmagentclient.go index 930845b3fc..7a4d3a39ad 100644 --- a/cns/nmagentclient/nmagentclient.go +++ b/cns/nmagentclient/nmagentclient.go @@ -35,7 +35,10 @@ type NMANetworkContainerResponse struct { type NMAgentSupportedApisResponseXML struct { SupportedApis []string `xml:"type"` } +<<<<<<< HEAD +======= +>>>>>>> Resovle merge conflict from master. type ContainerInfo struct { NetworkContainerID string `json:"networkContainerId"` Version string `json:"version"` From 9ee26fdec21dcaa4297d0b5dacd22a246aff6017 Mon Sep 17 00:00:00 2001 From: Shufang Date: Tue, 24 Nov 2020 15:38:28 -0800 Subject: [PATCH 08/12] Debug and it pass all the test. This is the final version. Change the way of http get request to add context. Change channel to no buffer with same goroutine. Found always fall in ctx.Done() condition. Add channel close for get nc version list. Add milisecond unit for timeout. Testing with different context version. --- cns/fakes/cnsfake.go | 2 +- cns/restserver/internalapi.go | 8 +++++--- cns/restserver/internalapi_test.go | 4 ++-- cns/service/main.go | 5 ++--- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/cns/fakes/cnsfake.go b/cns/fakes/cnsfake.go index 1d62b11f80..8cccf22157 100644 --- a/cns/fakes/cnsfake.go +++ b/cns/fakes/cnsfake.go @@ -233,7 +233,7 @@ func (fake *HTTPServiceFake) SyncNodeStatus(string, string, string, json.RawMess } // SyncHostNCVersion will update HostNCVersion in containerstatus. -func (fake *HTTPServiceFake) SyncHostNCVersion(context.Context, string, time.Time, time.Duration) { +func (fake *HTTPServiceFake) SyncHostNCVersion(context.Context, string, time.Time, time.Duration, time.Duration) { return } diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 8318ff74a3..31d8ecfe62 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -148,7 +148,7 @@ func (service *HTTPRestService) SyncNodeStatus(dncEP, infraVnet, nodeID string, // SyncHostNCVersion will check NC version from NMAgent and save it as host NC version in container status. // If NMAgent NC version got updated, CNS will refresh the pending programming IP status. -func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMode string, lastUpdatedHostNCTimeStamp time.Time, forceMarkIPAvailableTimeRange time.Duration) { +func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMode string, lastUpdatedHostNCTimeStamp time.Time, forceMarkIPAvailableTimeRange, syncHostNCTimeoutMilliSec time.Duration) { var hostVersionNeedUpdateNcList []string service.RLock() for _, containerstatus := range service.state.ContainerStatus { @@ -170,9 +170,11 @@ func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMo } service.RUnlock() if len(hostVersionNeedUpdateNcList) > 0 { - c1 := make(chan map[string]int, 1) + c1 := make(chan map[string]int) + ctxWithTimeout, _ := context.WithTimeout(ctx, syncHostNCTimeoutMilliSec*time.Millisecond) go func() { c1 <- service.nmagentClient.GetNcVersionListWithOutToken(hostVersionNeedUpdateNcList) + close(c1) }() select { case newHostNCVersionList := <-c1: @@ -200,7 +202,7 @@ func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMo lastUpdatedHostNCTimeStamp = time.Now() } service.Unlock() - case <-ctx.Done(): + case <-ctxWithTimeout.Done(): logger.Errorf("Timeout when getting vfp programmed NC version list from url without token") } } else { diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 4a441b4d6a..f685d42833 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -145,7 +145,7 @@ func testSyncHostNCVersion(t *testing.T, orchestratorType string) { t.Errorf("Unexpected containerStatus.DncNCVersion %s, expeted VM version should be 0 in string", containerStatus.DncNCVersion) } // When sync host NC version, it will use the orchestratorType pass in. - svc.SyncHostNCVersion(context.Background(), orchestratorType, time.Now(), 5*time.Second) + svc.SyncHostNCVersion(context.Background(), orchestratorType, time.Now(), 5*time.Second, 500*time.Millisecond) containerStatus = svc.state.ContainerStatus[req.NetworkContainerid] if containerStatus.HostNCVersion != "0" { t.Errorf("Unexpected containerStatus.HostNCVersion %s, expeted host version should be 0 in string", containerStatus.HostNCVersion) @@ -169,7 +169,7 @@ func TestPendingIPsGotUpdatedWhenSyncHostNCVersion(t *testing.T) { t.Errorf("Unexpected State %s, expeted State is %s, received %s, IP address is %s", podIPConfigState.State, cns.PendingProgramming, podIPConfigState.State, podIPConfigState.IPAddress) } } - svc.SyncHostNCVersion(context.Background(), cns.CRD, time.Now(), 5*time.Second) + svc.SyncHostNCVersion(context.Background(), cns.CRD, time.Now(), 5*time.Second, 500*time.Millisecond) containerStatus = svc.state.ContainerStatus[req.NetworkContainerid] receivedSecondaryIPConfigs = containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs diff --git a/cns/service/main.go b/cns/service/main.go index 64454a267d..3fc76361b4 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -570,12 +570,11 @@ func main() { logger.Printf("Starting SyncHostNCVersion") lastUpdatedHostNCTimeStamp := time.Now() rootCxt := context.Background() - ctxWithTimeout, _ := context.WithTimeout(rootCxt, cnsconfig.SyncHostNCTimeoutMilliSec) go func() { // Periodically poll vfp programmed NC version from NMAgent for { - <-time.NewTicker(cnsconfig.SyncHostNCVersionIntervalMilliSec * time.Second).C - httpRestServiceImplementation.SyncHostNCVersion(ctxWithTimeout, config.ChannelMode, lastUpdatedHostNCTimeStamp, cnsconfig.ForceMarkIPAvailableTimeRange) + <-time.NewTicker(cnsconfig.SyncHostNCVersionIntervalMilliSec * time.Millisecond).C + httpRestServiceImplementation.SyncHostNCVersion(rootCxt, config.ChannelMode, lastUpdatedHostNCTimeStamp, cnsconfig.ForceMarkIPAvailableTimeRange, cnsconfig.SyncHostNCTimeoutMilliSec) } }() From 7b4d200cc1cb6419ef44dc850ed2549c19d14176 Mon Sep 17 00:00:00 2001 From: Shufang Date: Wed, 25 Nov 2020 09:39:40 -0800 Subject: [PATCH 09/12] Resolve merge conflict. --- cns/nmagentclient/nmagentclient.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/cns/nmagentclient/nmagentclient.go b/cns/nmagentclient/nmagentclient.go index 7a4d3a39ad..930845b3fc 100644 --- a/cns/nmagentclient/nmagentclient.go +++ b/cns/nmagentclient/nmagentclient.go @@ -35,10 +35,7 @@ type NMANetworkContainerResponse struct { type NMAgentSupportedApisResponseXML struct { SupportedApis []string `xml:"type"` } -<<<<<<< HEAD -======= ->>>>>>> Resovle merge conflict from master. type ContainerInfo struct { NetworkContainerID string `json:"networkContainerId"` Version string `json:"version"` From 02f2800d73239504aac40efe717f342f08d4d40b Mon Sep 17 00:00:00 2001 From: Shufang Date: Wed, 25 Nov 2020 23:26:03 -0800 Subject: [PATCH 10/12] Remove force update pending programming IP to available logic. Remain retry if no response from NMAgent. Release pending programming IP when scale down. --- cns/configuration/configuration.go | 2 -- cns/fakes/cnsfake.go | 2 +- cns/requestcontroller/kubecontroller/crdtranslator.go | 4 ++-- cns/restserver/internalapi.go | 10 +--------- cns/restserver/internalapi_test.go | 4 ++-- cns/restserver/ipam.go | 2 +- cns/service/main.go | 3 +-- 7 files changed, 8 insertions(+), 19 deletions(-) diff --git a/cns/configuration/configuration.go b/cns/configuration/configuration.go index a824d1231f..866c802920 100644 --- a/cns/configuration/configuration.go +++ b/cns/configuration/configuration.go @@ -30,7 +30,6 @@ type CNSConfig struct { SyncHostNCVersionIntervalSec int SyncHostNCVersionIntervalMilliSec time.Duration SyncHostNCTimeoutMilliSec time.Duration - ForceMarkIPAvailableTimeRange time.Duration } type TelemetrySettings struct { @@ -136,5 +135,4 @@ func SetCNSConfigDefaults(config *CNSConfig) { } config.SyncHostNCVersionIntervalMilliSec = 1000 config.SyncHostNCTimeoutMilliSec = 500 - config.ForceMarkIPAvailableTimeRange = 5000 } diff --git a/cns/fakes/cnsfake.go b/cns/fakes/cnsfake.go index 8cccf22157..43f547a0f1 100644 --- a/cns/fakes/cnsfake.go +++ b/cns/fakes/cnsfake.go @@ -233,7 +233,7 @@ func (fake *HTTPServiceFake) SyncNodeStatus(string, string, string, json.RawMess } // SyncHostNCVersion will update HostNCVersion in containerstatus. -func (fake *HTTPServiceFake) SyncHostNCVersion(context.Context, string, time.Time, time.Duration, time.Duration) { +func (fake *HTTPServiceFake) SyncHostNCVersion(context.Context, string, time.Duration) { return } diff --git a/cns/requestcontroller/kubecontroller/crdtranslator.go b/cns/requestcontroller/kubecontroller/crdtranslator.go index fdca0020e0..471468bf9a 100644 --- a/cns/requestcontroller/kubecontroller/crdtranslator.go +++ b/cns/requestcontroller/kubecontroller/crdtranslator.go @@ -39,8 +39,8 @@ func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetw ncRequest.NetworkContainerid = nc.ID ncRequest.NetworkContainerType = cns.Docker ncRequest.Version = strconv.FormatInt(nc.Version, 10) - log.Printf("Set nc request info with SecondaryIPConfigs %v, NetworkContainerid %s, NetworkContainerType %s, NC Version %s", - ncRequest.SecondaryIPConfigs, ncRequest.NetworkContainerid, ncRequest.NetworkContainerType, ncRequest.Version) + log.Printf("Trying set nc request info with NetworkContainerid %s, NetworkContainerType %s, NC Version %s", + ncRequest.NetworkContainerid, ncRequest.NetworkContainerType, ncRequest.Version) if ip = net.ParseIP(nc.PrimaryIP); ip == nil { return ncRequest, fmt.Errorf("Invalid PrimaryIP %s:", nc.PrimaryIP) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 31d8ecfe62..38e16feeaa 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -148,7 +148,7 @@ func (service *HTTPRestService) SyncNodeStatus(dncEP, infraVnet, nodeID string, // SyncHostNCVersion will check NC version from NMAgent and save it as host NC version in container status. // If NMAgent NC version got updated, CNS will refresh the pending programming IP status. -func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMode string, lastUpdatedHostNCTimeStamp time.Time, forceMarkIPAvailableTimeRange, syncHostNCTimeoutMilliSec time.Duration) { +func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMode string, syncHostNCTimeoutMilliSec time.Duration) { var hostVersionNeedUpdateNcList []string service.RLock() for _, containerstatus := range service.state.ContainerStatus { @@ -181,11 +181,6 @@ func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMo service.Lock() if newHostNCVersionList == nil { logger.Errorf("Can't get vfp programmed NC version list from url without token") - if channelMode == cns.CRD && time.Since(lastUpdatedHostNCTimeStamp) > forceMarkIPAvailableTimeRange { - service.MarkAllPendingProgrammingIpsAsAvailableUntransacted() - lastUpdatedHostNCTimeStamp = time.Now() - logger.Printf("Can't get vfp programmed NC version list from url without token and performed force update.") - } } else { for ncID, newHostNCVersion := range newHostNCVersionList { // Check whether it exist in service state and get the related nc info @@ -199,14 +194,11 @@ func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMo service.state.ContainerStatus[ncID] = ncInfo } } - lastUpdatedHostNCTimeStamp = time.Now() } service.Unlock() case <-ctxWithTimeout.Done(): logger.Errorf("Timeout when getting vfp programmed NC version list from url without token") } - } else { - lastUpdatedHostNCTimeStamp = time.Now() } } diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index f685d42833..904f4cde06 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -145,7 +145,7 @@ func testSyncHostNCVersion(t *testing.T, orchestratorType string) { t.Errorf("Unexpected containerStatus.DncNCVersion %s, expeted VM version should be 0 in string", containerStatus.DncNCVersion) } // When sync host NC version, it will use the orchestratorType pass in. - svc.SyncHostNCVersion(context.Background(), orchestratorType, time.Now(), 5*time.Second, 500*time.Millisecond) + svc.SyncHostNCVersion(context.Background(), orchestratorType, 500*time.Millisecond) containerStatus = svc.state.ContainerStatus[req.NetworkContainerid] if containerStatus.HostNCVersion != "0" { t.Errorf("Unexpected containerStatus.HostNCVersion %s, expeted host version should be 0 in string", containerStatus.HostNCVersion) @@ -169,7 +169,7 @@ func TestPendingIPsGotUpdatedWhenSyncHostNCVersion(t *testing.T) { t.Errorf("Unexpected State %s, expeted State is %s, received %s, IP address is %s", podIPConfigState.State, cns.PendingProgramming, podIPConfigState.State, podIPConfigState.IPAddress) } } - svc.SyncHostNCVersion(context.Background(), cns.CRD, time.Now(), 5*time.Second, 500*time.Millisecond) + svc.SyncHostNCVersion(context.Background(), cns.CRD, 500*time.Millisecond) containerStatus = svc.state.ContainerStatus[req.NetworkContainerid] receivedSecondaryIPConfigs = containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index c55241dcaa..12669cc558 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -100,7 +100,7 @@ func (service *HTTPRestService) MarkIPsAsPending(numberToMark int) (map[string]c defer service.Unlock() for uuid, _ := range service.PodIPConfigState { mutableIPConfig := service.PodIPConfigState[uuid] - if mutableIPConfig.State == cns.Available { + if mutableIPConfig.State == cns.Available || mutableIPConfig.State == cns.PendingProgramming { mutableIPConfig.State = cns.PendingRelease service.PodIPConfigState[uuid] = mutableIPConfig pendingReleaseIPs[uuid] = mutableIPConfig diff --git a/cns/service/main.go b/cns/service/main.go index 3fc76361b4..715bed7297 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -568,13 +568,12 @@ func main() { } logger.Printf("Starting SyncHostNCVersion") - lastUpdatedHostNCTimeStamp := time.Now() rootCxt := context.Background() go func() { // Periodically poll vfp programmed NC version from NMAgent for { <-time.NewTicker(cnsconfig.SyncHostNCVersionIntervalMilliSec * time.Millisecond).C - httpRestServiceImplementation.SyncHostNCVersion(rootCxt, config.ChannelMode, lastUpdatedHostNCTimeStamp, cnsconfig.ForceMarkIPAvailableTimeRange, cnsconfig.SyncHostNCTimeoutMilliSec) + httpRestServiceImplementation.SyncHostNCVersion(rootCxt, config.ChannelMode, cnsconfig.SyncHostNCTimeoutMilliSec) } }() From d494cf6d155859117fcffa3498012ba52a0d3b5f Mon Sep 17 00:00:00 2001 From: Shufang Date: Mon, 30 Nov 2020 17:21:08 -0800 Subject: [PATCH 11/12] Remain VMVersion, HostVersion variable name as it is and use the Version inside CreateNetworkContainerRequest. --- cns/fakes/cnsfake.go | 2 +- .../kubecontroller/crdtranslator.go | 4 +-- cns/restserver/api.go | 4 +-- cns/restserver/const.go | 2 +- cns/restserver/internalapi.go | 12 ++++---- cns/restserver/internalapi_test.go | 20 ++++++------- cns/restserver/ipam.go | 4 +-- cns/restserver/restserver.go | 4 +-- cns/restserver/util.go | 30 +++++++++---------- 9 files changed, 41 insertions(+), 41 deletions(-) diff --git a/cns/fakes/cnsfake.go b/cns/fakes/cnsfake.go index 43f547a0f1..3177b1d679 100644 --- a/cns/fakes/cnsfake.go +++ b/cns/fakes/cnsfake.go @@ -232,7 +232,7 @@ func (fake *HTTPServiceFake) SyncNodeStatus(string, string, string, json.RawMess return 0, "" } -// SyncHostNCVersion will update HostNCVersion in containerstatus. +// SyncHostNCVersion will update HostVersion in containerstatus. func (fake *HTTPServiceFake) SyncHostNCVersion(context.Context, string, time.Duration) { return } diff --git a/cns/requestcontroller/kubecontroller/crdtranslator.go b/cns/requestcontroller/kubecontroller/crdtranslator.go index 471468bf9a..4c36db7349 100644 --- a/cns/requestcontroller/kubecontroller/crdtranslator.go +++ b/cns/requestcontroller/kubecontroller/crdtranslator.go @@ -39,8 +39,6 @@ func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetw ncRequest.NetworkContainerid = nc.ID ncRequest.NetworkContainerType = cns.Docker ncRequest.Version = strconv.FormatInt(nc.Version, 10) - log.Printf("Trying set nc request info with NetworkContainerid %s, NetworkContainerType %s, NC Version %s", - ncRequest.NetworkContainerid, ncRequest.NetworkContainerType, ncRequest.Version) if ip = net.ParseIP(nc.PrimaryIP); ip == nil { return ncRequest, fmt.Errorf("Invalid PrimaryIP %s:", nc.PrimaryIP) @@ -71,6 +69,8 @@ func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetw ncRequest.SecondaryIPConfigs[ipAssignment.Name] = secondaryIPConfig log.Debugf("Seconday IP Configs got set, name is %s, config is %v", ipAssignment.Name, secondaryIPConfig) } + log.Printf("Set nc request info with NetworkContainerid %s, NetworkContainerType %s, NC Version %s", + ncRequest.NetworkContainerid, ncRequest.NetworkContainerType, ncRequest.Version) } //Only returning the first network container for now, later we will return a list diff --git a/cns/restserver/api.go b/cns/restserver/api.go index f5412caec3..bbcd719f4b 100644 --- a/cns/restserver/api.go +++ b/cns/restserver/api.go @@ -786,7 +786,7 @@ func (service *HTTPRestService) createOrUpdateNetworkContainer(w http.ResponseWr existing, ok := service.getNetworkContainerDetails(req.NetworkContainerid) // create/update nc only if it doesn't exist or it exists and the requested version is different from the saved version - if !ok || (ok && existing.DncNCVersion != req.Version) { + if !ok || (ok && existing.VMVersion != req.Version) { nc := service.networkContainer if err = nc.Create(req); err != nil { returnMessage = fmt.Sprintf("[Azure CNS] Error. CreateOrUpdateNetworkContainer failed %v", err.Error()) @@ -799,7 +799,7 @@ func (service *HTTPRestService) createOrUpdateNetworkContainer(w http.ResponseWr existing, ok := service.getNetworkContainerDetails(req.NetworkContainerid) // create/update nc only if it doesn't exist or it exists and the requested version is different from the saved version - if ok && existing.DncNCVersion != req.Version { + if ok && existing.VMVersion != req.Version { nc := service.networkContainer netPluginConfig := service.getNetPluginDetails() if err = nc.Update(req, netPluginConfig); err != nil { diff --git a/cns/restserver/const.go b/cns/restserver/const.go index bc868e8e4a..a1c7253f6c 100644 --- a/cns/restserver/const.go +++ b/cns/restserver/const.go @@ -38,7 +38,7 @@ const ( NetworkContainerVfpProgramComplete = 35 NetworkContainerVfpProgramCheckSkipped = 36 NmAgentSupportedApisError = 37 - UnsupportedNCVersion = 37 + UnsupportedNCVersion = 38 UnexpectedError = 99 ) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 38e16feeaa..df94db0e38 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -153,18 +153,18 @@ func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMo service.RLock() for _, containerstatus := range service.state.ContainerStatus { // Will open a separate PR to convert all the NC version related variable to int. Change from string to int is a pain. - hostNcVersion, err := strconv.Atoi(containerstatus.HostNCVersion) + hostVersion, err := strconv.Atoi(containerstatus.HostVersion) if err != nil { - log.Errorf("Received err when change containerstatus.HostNCVersion %s to int, err msg %v", containerstatus.HostNCVersion, err) + log.Errorf("Received err when change containerstatus.HostVersion %s to int, err msg %v", containerstatus.HostVersion, err) return } - dncNcVersion, err := strconv.Atoi(containerstatus.DncNCVersion) + dncNcVersion, err := strconv.Atoi(containerstatus.CreateNetworkContainerRequest.Version) if err != nil { - log.Errorf("Received err when change containerstatus.DncNCVersion %s to int, err msg %v", containerstatus.DncNCVersion, err) + log.Errorf("Received err when change nc version %s in containerstatusto int, err msg %v", containerstatus.CreateNetworkContainerRequest.Version, err) return } // host NC version is the NC version from NMAgent, if it's smaller than - if hostNcVersion < dncNcVersion { + if hostVersion < dncNcVersion { hostVersionNeedUpdateNcList = append(hostVersionNeedUpdateNcList, containerstatus.ID) } } @@ -190,7 +190,7 @@ func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMo if channelMode == cns.CRD { service.MarkIpsAsAvailableUntransacted(ncInfo.ID, newHostNCVersion) } - ncInfo.HostNCVersion = strconv.Itoa(newHostNCVersion) + ncInfo.HostVersion = strconv.Itoa(newHostNCVersion) service.state.ContainerStatus[ncID] = ncInfo } } diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 904f4cde06..1b43fb4826 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -138,20 +138,20 @@ func TestSyncHostNCVersion(t *testing.T) { func testSyncHostNCVersion(t *testing.T, orchestratorType string) { req := createNCReqeustForSyncHostNCVersion(t) containerStatus := svc.state.ContainerStatus[req.NetworkContainerid] - if containerStatus.HostNCVersion != "-1" { - t.Errorf("Unexpected containerStatus.HostNCVersion %s, expeted host version should be -1 in string", containerStatus.HostNCVersion) + if containerStatus.HostVersion != "-1" { + t.Errorf("Unexpected containerStatus.HostVersion %s, expeted host version should be -1 in string", containerStatus.HostVersion) } - if containerStatus.DncNCVersion != "0" { - t.Errorf("Unexpected containerStatus.DncNCVersion %s, expeted VM version should be 0 in string", containerStatus.DncNCVersion) + if containerStatus.CreateNetworkContainerRequest.Version != "0" { + t.Errorf("Unexpected nc version in containerStatus as %s, expeted VM version should be 0 in string", containerStatus.CreateNetworkContainerRequest.Version) } // When sync host NC version, it will use the orchestratorType pass in. svc.SyncHostNCVersion(context.Background(), orchestratorType, 500*time.Millisecond) containerStatus = svc.state.ContainerStatus[req.NetworkContainerid] - if containerStatus.HostNCVersion != "0" { - t.Errorf("Unexpected containerStatus.HostNCVersion %s, expeted host version should be 0 in string", containerStatus.HostNCVersion) + if containerStatus.HostVersion != "0" { + t.Errorf("Unexpected containerStatus.HostVersion %s, expeted host version should be 0 in string", containerStatus.HostVersion) } - if containerStatus.DncNCVersion != "0" { - t.Errorf("Unexpected containerStatus.DncNCVersion %s, expeted VM version should be 0 in string", containerStatus.DncNCVersion) + if containerStatus.CreateNetworkContainerRequest.Version != "0" { + t.Errorf("Unexpected nc version in containerStatus as %s, expeted VM version should be 0 in string", containerStatus.CreateNetworkContainerRequest.Version) } } @@ -397,12 +397,12 @@ func validateNetworkRequest(t *testing.T, req cns.CreateNetworkContainerRequest) var expectedIPStatus string // 0 is the default NMAgent version return from fake GetNetworkContainerInfoFromHost - if containerStatus.DncNCVersion > "0" { + if containerStatus.CreateNetworkContainerRequest.Version > "0" { expectedIPStatus = cns.PendingProgramming } else { expectedIPStatus = cns.Available } - t.Logf("DncNCVersion is %s, HostNCVersion is %s", containerStatus.DncNCVersion, containerStatus.HostNCVersion) + t.Logf("NC version in container status is %s, HostVersion is %s", containerStatus.CreateNetworkContainerRequest.Version, containerStatus.HostVersion) var alreadyValidated = make(map[string]string) for ipid, ipStatus := range svc.PodIPConfigState { if ipaddress, found := alreadyValidated[ipid]; !found { diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 12669cc558..34b40efc05 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -133,9 +133,9 @@ func (service *HTTPRestService) MarkIpsAsAvailableUntransacted(ncID string, newH if ncInfo, exist := service.state.ContainerStatus[ncID]; !exist { logger.Errorf("Can't find NC with ID %s in service state, stop updating its pending programming IP status", ncID) } else { - previousHostNCVersion := ncInfo.HostNCVersion + previousHostNCVersion := ncInfo.HostVersion // We only need to handle the situation when dnc nc version is larger than programmed nc version - if previousHostNCVersion < ncInfo.DncNCVersion && previousHostNCVersion < strconv.Itoa(newHostNCVersion) { + if previousHostNCVersion < ncInfo.CreateNetworkContainerRequest.Version && previousHostNCVersion < strconv.Itoa(newHostNCVersion) { for uuid, secondaryIPConfigs := range ncInfo.CreateNetworkContainerRequest.SecondaryIPConfigs { if ipConfigStatus, exist := service.PodIPConfigState[uuid]; !exist { logger.Errorf("IP %s with uuid as %s exist in service state Secondary IP list but can't find in PodIPConfigState", ipConfigStatus.IPAddress, uuid) diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index bd9b1df468..ca325db8b2 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -59,8 +59,8 @@ type allocatedIPCount struct { // containerstatus is used to save status of an existing container type containerstatus struct { ID string - DncNCVersion string - HostNCVersion string + VMVersion string + HostVersion string CreateNetworkContainerRequest cns.CreateNetworkContainerRequest VfpUpdateComplete bool // True when VFP programming is completed for the NC } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 8e28e82faa..5fcfda7dc1 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -106,7 +106,7 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw defer service.Unlock() var ( - hostNCVersion string + hostVersion string existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig //uuid is key vfpUpdateComplete bool ) @@ -117,14 +117,14 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw existingNCStatus, ok := service.state.ContainerStatus[req.NetworkContainerid] if ok { - hostNCVersion = existingNCStatus.HostNCVersion + hostVersion = existingNCStatus.HostVersion existingSecondaryIPConfigs = existingNCStatus.CreateNetworkContainerRequest.SecondaryIPConfigs vfpUpdateComplete = existingNCStatus.VfpUpdateComplete } - if hostNCVersion == "" { + if hostVersion == "" { // Host version is the NC version from NMAgent, set it -1 to indicate no result from NMAgent yet. // TODO, query NMAgent and with aggresive time out and assign latest host version. - hostNCVersion = "-1" + hostVersion = "-1" } // Remove the auth token before saving the containerStatus to cns json file @@ -134,9 +134,9 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw service.state.ContainerStatus[req.NetworkContainerid] = containerstatus{ ID: req.NetworkContainerid, + VMVersion: req.Version, CreateNetworkContainerRequest: createNetworkContainerRequest, - DncNCVersion: req.Version, - HostNCVersion: hostNCVersion, + HostVersion: hostVersion, VfpUpdateComplete: vfpUpdateComplete} switch req.NetworkContainerType { @@ -181,7 +181,7 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw case cns.KubernetesCRD: // Validate and Update the SecondaryIpConfig state - returnCode, returnMesage := service.updateIpConfigsStateUntransacted(req, existingSecondaryIPConfigs, hostNCVersion) + returnCode, returnMesage := service.updateIpConfigsStateUntransacted(req, existingSecondaryIPConfigs, hostVersion) if returnCode != 0 { return returnCode, returnMesage } @@ -201,9 +201,9 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw return 0, "" } -// This func will compute the deltaIpConfigState which needs to be 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, hostNCVersion string) (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) @@ -246,8 +246,8 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN // TODO, will udpate NC version related variable to int, change it from string to int is a pains var hostNCVersionInInt int var err error - if hostNCVersionInInt, err = strconv.Atoi(hostNCVersion); err != nil { - return UnsupportedNCVersion, fmt.Sprintf("Invalid hostNCVersion is %s, err:%s", hostNCVersion, err) + if hostNCVersionInInt, err = strconv.Atoi(hostVersion); err != nil { + return UnsupportedNCVersion, fmt.Sprintf("Invalid hostVersion is %s, err:%s", hostVersion, err) } service.addIPConfigStateUntransacted(req.NetworkContainerid, hostNCVersionInInt, req.SecondaryIPConfigs, existingSecondaryIPConfigs) @@ -257,7 +257,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(ncId string, hostNCVersion int, ipconfigs, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig) { +func (service *HTTPRestService) addIPConfigStateUntransacted(ncId string, hostVersion int, ipconfigs, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig) { newIPCNSStatus := cns.Available // add ipconfigs to state for ipId, ipconfig := range ipconfigs { @@ -266,15 +266,15 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(ncId string, hostNC if existingIPConfig, existsInPreviousIPConfig := existingSecondaryIPConfigs[ipId]; existsInPreviousIPConfig { ipconfig.NCVersion = existingIPConfig.NCVersion ipconfigs[ipId] = ipconfig - hostNCVersion = existingIPConfig.NCVersion + hostVersion = existingIPConfig.NCVersion } - logger.Printf("[Azure-Cns] Set IP %s version to %d, programmed host nc version is %d", ipconfig.IPAddress, ipconfig.NCVersion, hostNCVersion) + logger.Printf("[Azure-Cns] Set IP %s version to %d, programmed host nc version is %d", ipconfig.IPAddress, ipconfig.NCVersion, hostVersion) if _, exists := service.PodIPConfigState[ipId]; exists { continue } // Using the updated NC version attached with IP to compare with latest nmagent version and determine IP statues. // When reconcile, service.PodIPConfigState doens't exist, rebuild it with the help of NC version attached with IP. - if hostNCVersion < ipconfig.NCVersion { + if hostVersion < ipconfig.NCVersion { newIPCNSStatus = cns.PendingProgramming } // add the new State From 2f2cc8052dec24bd1b7c4f678007d7d8db2feea5 Mon Sep 17 00:00:00 2001 From: Shufang Date: Wed, 9 Dec 2020 17:23:56 -0800 Subject: [PATCH 12/12] Addressed team member feedback. --- cns/configuration/configuration.go | 27 +++++++++---------- cns/fakes/nmagentclientfake.go | 2 +- cns/nmagentclient/nmagentclient.go | 14 +++++----- .../kubecontroller/crdtranslator.go | 2 +- cns/restserver/internalapi.go | 22 ++++++++------- cns/restserver/ipam.go | 12 --------- cns/service/main.go | 6 ++--- 7 files changed, 37 insertions(+), 48 deletions(-) diff --git a/cns/configuration/configuration.go b/cns/configuration/configuration.go index 866c802920..40111ae1f2 100644 --- a/cns/configuration/configuration.go +++ b/cns/configuration/configuration.go @@ -18,18 +18,17 @@ const ( ) type CNSConfig struct { - TelemetrySettings TelemetrySettings - ManagedSettings ManagedSettings - ChannelMode string - UseHTTPS bool - TLSSubjectName string - TLSCertificatePath string - TLSPort string - TLSEndpoint string - WireserverIP string - SyncHostNCVersionIntervalSec int - SyncHostNCVersionIntervalMilliSec time.Duration - SyncHostNCTimeoutMilliSec time.Duration + TelemetrySettings TelemetrySettings + ManagedSettings ManagedSettings + ChannelMode string + UseHTTPS bool + TLSSubjectName string + TLSCertificatePath string + TLSPort string + TLSEndpoint string + WireserverIP string + SyncHostNCVersionIntervalMs time.Duration + SyncHostNCTimeoutMs time.Duration } type TelemetrySettings struct { @@ -133,6 +132,6 @@ func SetCNSConfigDefaults(config *CNSConfig) { if config.ChannelMode == "" { config.ChannelMode = cns.Direct } - config.SyncHostNCVersionIntervalMilliSec = 1000 - config.SyncHostNCTimeoutMilliSec = 500 + config.SyncHostNCVersionIntervalMs = 1000 + config.SyncHostNCTimeoutMs = 500 } diff --git a/cns/fakes/nmagentclientfake.go b/cns/fakes/nmagentclientfake.go index e61a104dc2..5cf601f7b1 100644 --- a/cns/fakes/nmagentclientfake.go +++ b/cns/fakes/nmagentclientfake.go @@ -1,4 +1,4 @@ -// Copyright 2017 Microsoft. All rights reserved. +// Copyright 2020 Microsoft. All rights reserved. // MIT License package fakes diff --git a/cns/nmagentclient/nmagentclient.go b/cns/nmagentclient/nmagentclient.go index 930845b3fc..26135a3650 100644 --- a/cns/nmagentclient/nmagentclient.go +++ b/cns/nmagentclient/nmagentclient.go @@ -191,7 +191,7 @@ func (nmagentclient *NMAgentClient) GetNcVersionListWithOutToken(ncNeedUpdateLis now := time.Now() response, err := http.Get(nmagentclient.connectionURL) latency := time.Since(now) - logger.Printf("[NMAgentClient][Response] GetNcVersionListWithOutToken response: %+v. QueryURL is %s, latency is %v", response, nmagentclient.connectionURL, latency) + logger.Printf("[NMAgentClient][Response] GetNcVersionListWithOutToken response: %+v, latency is %d", response, latency.Milliseconds()) if response.StatusCode != http.StatusOK { logger.Printf("[NMAgentClient][Response] GetNcVersionListWithOutToken failed with %d, err is %v", response.StatusCode, err) @@ -202,7 +202,7 @@ func (nmagentclient *NMAgentClient) GetNcVersionListWithOutToken(ncNeedUpdateLis rBytes, _ := ioutil.ReadAll(response.Body) logger.Printf("Response body is %v", rBytes) json.Unmarshal(rBytes, &nmaNcListResponse) - if nmaNcListResponse.ResponseCode != "200" { + if nmaNcListResponse.ResponseCode != strconv.Itoa(http.StatusOK) { logger.Printf("[NMAgentClient][Response] GetNcVersionListWithOutToken unmarshal failed with %s", rBytes) return nil } @@ -212,12 +212,12 @@ func (nmagentclient *NMAgentClient) GetNcVersionListWithOutToken(ncNeedUpdateLis receivedNcVersionListInMap[containers.NetworkContainerID] = containers.Version } for _, ncID := range ncNeedUpdateList { - if val, ok := receivedNcVersionListInMap[ncID]; ok { - if valInInt, err := strconv.Atoi(val); err != nil { - logger.Printf("[NMAgentClient][Response] GetNcVersionListWithOutToken translate version %s to int failed with %s", val, err) + if version, ok := receivedNcVersionListInMap[ncID]; ok { + if versionInInt, err := strconv.Atoi(version); err != nil { + logger.Printf("[NMAgentClient][Response] GetNcVersionListWithOutToken translate version %s to int failed with %s", version, err) } else { - ncVersionList[ncID] = valInInt - logger.Printf("Containers id is %s, version is %d", ncID, valInInt) + ncVersionList[ncID] = versionInInt + logger.Printf("Containers id is %s, programmed NC version is %d", ncID, versionInInt) } } } diff --git a/cns/requestcontroller/kubecontroller/crdtranslator.go b/cns/requestcontroller/kubecontroller/crdtranslator.go index 4c36db7349..95f184e257 100644 --- a/cns/requestcontroller/kubecontroller/crdtranslator.go +++ b/cns/requestcontroller/kubecontroller/crdtranslator.go @@ -69,7 +69,7 @@ func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetw ncRequest.SecondaryIPConfigs[ipAssignment.Name] = secondaryIPConfig log.Debugf("Seconday IP Configs got set, name is %s, config is %v", ipAssignment.Name, secondaryIPConfig) } - log.Printf("Set nc request info with NetworkContainerid %s, NetworkContainerType %s, NC Version %s", + log.Printf("Set NC request info with NetworkContainerid %s, NetworkContainerType %s, NC Version %s", ncRequest.NetworkContainerid, ncRequest.NetworkContainerType, ncRequest.Version) } diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index df94db0e38..f545dc329b 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -156,32 +156,34 @@ func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMo hostVersion, err := strconv.Atoi(containerstatus.HostVersion) if err != nil { log.Errorf("Received err when change containerstatus.HostVersion %s to int, err msg %v", containerstatus.HostVersion, err) - return + continue } dncNcVersion, err := strconv.Atoi(containerstatus.CreateNetworkContainerRequest.Version) if err != nil { - log.Errorf("Received err when change nc version %s in containerstatusto int, err msg %v", containerstatus.CreateNetworkContainerRequest.Version, err) - return + log.Errorf("Received err when change nc version %s in containerstatus to int, err msg %v", containerstatus.CreateNetworkContainerRequest.Version, err) + continue } - // host NC version is the NC version from NMAgent, if it's smaller than + // host NC version is the NC version from NMAgent, if it's smaller than NC version from DNC, then append it to indicate it needs update. if hostVersion < dncNcVersion { hostVersionNeedUpdateNcList = append(hostVersionNeedUpdateNcList, containerstatus.ID) + } else if hostVersion > dncNcVersion { + log.Errorf("NC version from NMAgent is larger than DNC, NC version from NMAgent is %d, NC version from DNC is %d", hostVersion, dncNcVersion) } } service.RUnlock() if len(hostVersionNeedUpdateNcList) > 0 { - c1 := make(chan map[string]int) + ncVersionChannel := make(chan map[string]int) ctxWithTimeout, _ := context.WithTimeout(ctx, syncHostNCTimeoutMilliSec*time.Millisecond) go func() { - c1 <- service.nmagentClient.GetNcVersionListWithOutToken(hostVersionNeedUpdateNcList) - close(c1) + ncVersionChannel <- service.nmagentClient.GetNcVersionListWithOutToken(hostVersionNeedUpdateNcList) + close(ncVersionChannel) }() select { - case newHostNCVersionList := <-c1: - service.Lock() + case newHostNCVersionList := <-ncVersionChannel: if newHostNCVersionList == nil { logger.Errorf("Can't get vfp programmed NC version list from url without token") } else { + service.Lock() for ncID, newHostNCVersion := range newHostNCVersionList { // Check whether it exist in service state and get the related nc info if ncInfo, exist := service.state.ContainerStatus[ncID]; !exist { @@ -194,8 +196,8 @@ func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMo service.state.ContainerStatus[ncID] = ncInfo } } + service.Unlock() } - service.Unlock() case <-ctxWithTimeout.Done(): logger.Errorf("Timeout when getting vfp programmed NC version list from url without token") } diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 34b40efc05..843971a207 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -114,18 +114,6 @@ func (service *HTTPRestService) MarkIPsAsPending(numberToMark int) (map[string]c return nil, fmt.Errorf("Failed to mark %d IP's as pending, only marked %d IP's", numberToMark, len(pendingReleaseIPs)) } -// MarkAllPendingProgrammingIpsAsAvailableUntransacted is the function to update pending programming IPs to available -// when get NC version failed and we don't want to block IP allocation. -// Note: this func is an untransacted API as the caller will take a Service lock -func (service *HTTPRestService) MarkAllPendingProgrammingIpsAsAvailableUntransacted() { - for _, ipConfigStatus := range service.PodIPConfigState { - if ipConfigStatus.State == cns.PendingProgramming { - ipConfigStatus.State = cns.Available - service.PodIPConfigState[ipConfigStatus.ID] = ipConfigStatus - } - } -} - // MarkIpsAsAvailableUntransacted will update pending programming IPs to available if NMAgent side's programmed nc version keep up with nc version. // Note: this func is an untransacted API as the caller will take a Service lock func (service *HTTPRestService) MarkIpsAsAvailableUntransacted(ncID string, newHostNCVersion int) { diff --git a/cns/service/main.go b/cns/service/main.go index 715bed7297..0cd8696ffa 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -455,7 +455,7 @@ func main() { nmaclient, err := nmagentclient.NewNMAgentClient("") if err != nil { - logger.Errorf("Failed to start nmagent client due to error %v\n", err) + logger.Errorf("Failed to start nmagent client due to error %v", err) return } // Create CNS object. @@ -572,8 +572,8 @@ func main() { go func() { // Periodically poll vfp programmed NC version from NMAgent for { - <-time.NewTicker(cnsconfig.SyncHostNCVersionIntervalMilliSec * time.Millisecond).C - httpRestServiceImplementation.SyncHostNCVersion(rootCxt, config.ChannelMode, cnsconfig.SyncHostNCTimeoutMilliSec) + <-time.NewTicker(cnsconfig.SyncHostNCVersionIntervalMs * time.Millisecond).C + httpRestServiceImplementation.SyncHostNCVersion(rootCxt, config.ChannelMode, cnsconfig.SyncHostNCTimeoutMs) } }()