From 313cfe7cc6ffdc26a98501e4e22db66f9e28f990 Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Wed, 17 Aug 2022 11:06:18 -0700 Subject: [PATCH 01/16] rebase --- azure-ipam/ipconfig/ipconfig.go | 1 + cns/NetworkContainerContract.go | 2 +- cns/client/client_test.go | 2 +- cns/cnireconciler/podinfoprovider.go | 35 +++++++++++++++++ cns/configuration/configuration.go | 1 + cns/restserver/api_test.go | 2 +- cns/restserver/ipam.go | 59 ++++++++++++++++++++++++++++ cns/restserver/ipam_test.go | 2 +- cns/restserver/restserver.go | 14 ++++++- cns/service/main.go | 34 ++++++++++++++-- cns/types/codes.go | 1 + common/config.go | 3 ++ 12 files changed, 146 insertions(+), 10 deletions(-) diff --git a/azure-ipam/ipconfig/ipconfig.go b/azure-ipam/ipconfig/ipconfig.go index 6906f8cd72..a560d44743 100644 --- a/azure-ipam/ipconfig/ipconfig.go +++ b/azure-ipam/ipconfig/ipconfig.go @@ -32,6 +32,7 @@ func CreateIPConfigReq(args *cniSkel.CmdArgs) (cns.IPConfigRequest, error) { PodInterfaceID: args.ContainerID, InfraContainerID: args.ContainerID, OrchestratorContext: orchestratorContext, + Ifname: args.IfName, } return req, nil diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 644bc44bc3..d253eb0b19 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -390,7 +390,7 @@ type IPConfigRequest struct { PodInterfaceID string InfraContainerID string OrchestratorContext json.RawMessage - Ifname string + Ifname string // Used by delegated IPAM } func (i IPConfigRequest) String() string { diff --git a/cns/client/client_test.go b/cns/client/client_test.go index ae5deb929e..e1e03f7d55 100644 --- a/cns/client/client_test.go +++ b/cns/client/client_test.go @@ -169,7 +169,7 @@ func TestMain(m *testing.M) { logger.InitLogger(logName, 0, 0, tmpLogDir+"/") config := common.ServiceConfig{} - httpRestService, err := restserver.NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.NMAgentClientFake{}) + httpRestService, err := restserver.NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.NMAgentClientFake{}, nil) svc = httpRestService.(*restserver.HTTPRestService) svc.Name = "cns-test-server" fakeNNC := v1alpha.NodeNetworkConfig{ diff --git a/cns/cnireconciler/podinfoprovider.go b/cns/cnireconciler/podinfoprovider.go index 959ca64259..91d749e62e 100644 --- a/cns/cnireconciler/podinfoprovider.go +++ b/cns/cnireconciler/podinfoprovider.go @@ -6,6 +6,8 @@ import ( "github.com/Azure/azure-container-networking/cni/api" "github.com/Azure/azure-container-networking/cni/client" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/restserver" + "github.com/Azure/azure-container-networking/store" "github.com/pkg/errors" "k8s.io/utils/exec" ) @@ -16,6 +18,21 @@ func NewCNIPodInfoProvider() (cns.PodInfoByIPProvider, error) { return newCNIPodInfoProvider(exec.New()) } +func NewCNSPodInfoProvider(endpointStore store.KeyValueStore) (cns.PodInfoByIPProvider, error) { + return newCNSPodInfoProvider(endpointStore) +} + +func newCNSPodInfoProvider(endpointStore store.KeyValueStore) (cns.PodInfoByIPProvider, error) { + var state map[string]*restserver.EndpointInfo + err := endpointStore.Read("endpoints", state) + if err != nil { + return nil, fmt.Errorf("failed to read endpoints state from store : %w", err) + } + return cns.PodInfoByIPProviderFunc(func() (map[string]cns.PodInfo, error) { + return endpointStateToPodInfoByIP(state) + }), nil +} + func newCNIPodInfoProvider(exec exec.Interface) (cns.PodInfoByIPProvider, error) { cli := client.New(exec) state, err := cli.GetEndpointState() @@ -44,3 +61,21 @@ func cniStateToPodInfoByIP(state *api.AzureCNIState) (map[string]cns.PodInfo, er } return podInfoByIP, nil } + +func endpointStateToPodInfoByIP(state map[string]*restserver.EndpointInfo) (map[string]cns.PodInfo, error) { + podInfoByIP := map[string]cns.PodInfo{} + for containerID, endpointInfo := range state { + for _, ipnet := range endpointInfo.IfnameToIPMap { + if _, ok := podInfoByIP[ipnet.IP.String()]; ok { + return nil, errors.Wrap(cns.ErrDuplicateIP, ipnet.IP.String()) + } + podInfoByIP[ipnet.IP.String()] = cns.NewPodInfo( + containerID, + containerID, // decided to treated podInterfaceID and podContainerID to be the same + endpointInfo.PodName, + endpointInfo.PodNamespace, + ) + } + } + return podInfoByIP, nil +} diff --git a/cns/configuration/configuration.go b/cns/configuration/configuration.go index ddfcd2c5ba..4ce7cd1402 100644 --- a/cns/configuration/configuration.go +++ b/cns/configuration/configuration.go @@ -37,6 +37,7 @@ type CNSConfig struct { KeyVaultSettings KeyVaultSettings MSISettings MSISettings ProgramSNATIPTables bool + ManageEndpointState bool } type TelemetrySettings struct { diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index f6c7f2d39b..a9ecae7e2c 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -1078,7 +1078,7 @@ func startService() error { } nmagentClient := &fakes.NMAgentClientFake{} - service, err = NewHTTPRestService(&config, &fakes.WireserverClientFake{}, nmagentClient) + service, err = NewHTTPRestService(&config, &fakes.WireserverClientFake{}, nmagentClient, nil) if err != nil { return err } diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 1feead1487..e0a6455659 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -5,6 +5,7 @@ package restserver import ( "fmt" + "net" "net/http" "strconv" @@ -12,6 +13,7 @@ import ( "github.com/Azure/azure-container-networking/cns/filter" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/types" + "github.com/Azure/azure-container-networking/common" "github.com/pkg/errors" ) @@ -65,6 +67,25 @@ func (service *HTTPRestService) requestIPConfigHandler(w http.ResponseWriter, r ipAssignmentLatency.Observe(since.Seconds()) } }() + + // Check if http rest service managed endpoint state is set + if service.Options[common.OptManageEndpointState] == true { + err = service.updateEndpointState(ipconfigRequest, podInfo, podIPInfo) + if err != nil { + reserveResp := &cns.IPConfigResponse{ + Response: cns.Response{ + ReturnCode: types.UnexpectedError, + Message: fmt.Sprintf("Update endpoint state failed: %v ", err), + }, + PodIpInfo: podIPInfo, + } + w.Header().Set(cnsReturnCode, reserveResp.Response.ReturnCode.String()) + err = service.Listener.Encode(w, &reserveResp) + logger.ResponseEx(service.Name+operationName, ipconfigRequest, reserveResp, reserveResp.Response.ReturnCode, err) + return + } + } + reserveResp := &cns.IPConfigResponse{ Response: cns.Response{ ReturnCode: types.Success, @@ -76,6 +97,44 @@ func (service *HTTPRestService) requestIPConfigHandler(w http.ResponseWriter, r logger.ResponseEx(service.Name+operationName, ipconfigRequest, reserveResp, reserveResp.Response.ReturnCode, err) } +func (service *HTTPRestService) updateEndpointState(ipconfigRequest cns.IPConfigRequest, podInfo cns.PodInfo, podIPInfo cns.PodIpInfo) error { + if service.EndpointStateStore == nil { + return fmt.Errorf("nil endpoint state store") + } + service.Lock() + defer service.Unlock() + if service.EndpointState == nil { // if endpoint state is empty then read from store + err := service.EndpointStateStore.Read("endpoints", service.EndpointState) + if err != nil { + return fmt.Errorf("failed to read endpoint state from store: %w", err) + } + } + + if endpointInfo, ok := service.EndpointState[ipconfigRequest.InfraContainerID]; ok { + _, ipnet, err := net.ParseCIDR(podIPInfo.PodIPConfig.IPAddress + "/" + fmt.Sprint(podIPInfo.PodIPConfig.PrefixLength)) + if err != nil { + return fmt.Errorf("failed to parse pod ip address: %w", err) + } + endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname] = ipnet + service.EndpointState[ipconfigRequest.InfraContainerID] = endpointInfo + + } else { + endpointInfo := &EndpointInfo{PodName: podInfo.Name(), PodNamespace: podInfo.Namespace(), IfnameToIPMap: make(map[string]*net.IPNet)} + _, ipnet, err := net.ParseCIDR(podIPInfo.PodIPConfig.IPAddress + "/" + fmt.Sprint(podIPInfo.PodIPConfig.PrefixLength)) + if err != nil { + return fmt.Errorf("failed to parse pod ip address: %w", err) + } + endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname] = ipnet + service.EndpointState[ipconfigRequest.InfraContainerID] = endpointInfo + } + + err := service.EndpointStateStore.Write("endpoints", service.EndpointState) + if err != nil { + return fmt.Errorf("failed to write endpoint state to store: %w", err) + } + return nil +} + func (service *HTTPRestService) releaseIPConfigHandler(w http.ResponseWriter, r *http.Request) { var req cns.IPConfigRequest err := service.Listener.Decode(w, r, &req) diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index dc0a5f8e9c..35e77e68a0 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -37,7 +37,7 @@ var ( func getTestService() *HTTPRestService { var config common.ServiceConfig - httpsvc, _ := NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.NMAgentClientFake{}) + httpsvc, _ := NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.NMAgentClientFake{}, nil) svc = httpsvc.(*HTTPRestService) svc.IPAMPoolMonitor = &fakes.MonitorFake{} setOrchestratorTypeInternal(cns.KubernetesCRD) diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index d2e3ae8096..30bea98d8a 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -2,6 +2,7 @@ package restserver import ( "context" + "net" "sync" "time" @@ -58,7 +59,15 @@ type HTTPRestService struct { state *httpRestServiceState podsPendingIPAssignment *bounded.TimedSet sync.RWMutex - dncPartitionKey string + dncPartitionKey string + EndpointState map[string]*EndpointInfo // key : container ID + EndpointStateStore store.KeyValueStore +} + +type EndpointInfo struct { + PodName string + PodNamespace string + IfnameToIPMap map[string]*net.IPNet } type GetHTTPServiceDataResponse struct { @@ -109,7 +118,7 @@ type networkInfo struct { } // NewHTTPRestService creates a new HTTP Service object. -func NewHTTPRestService(config *common.ServiceConfig, wscli interfaceGetter, nmagentClient nmagentClient) (cns.HTTPService, error) { +func NewHTTPRestService(config *common.ServiceConfig, wscli interfaceGetter, nmagentClient nmagentClient, endpointStateStore store.KeyValueStore) (cns.HTTPService, error) { service, err := cns.NewService(config.Name, config.Version, config.ChannelMode, config.Store) if err != nil { return nil, err @@ -158,6 +167,7 @@ func NewHTTPRestService(config *common.ServiceConfig, wscli interfaceGetter, nma routingTable: routingTable, state: serviceState, podsPendingIPAssignment: bounded.NewTimedSet(250), // nolint:gomnd // maxpods + EndpointStateStore: endpointStateStore, }, nil } diff --git a/cns/service/main.go b/cns/service/main.go index 2f5fb76a96..e277a37016 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -438,8 +438,9 @@ func main() { // Initialize CNS. var ( - err error - config common.ServiceConfig + err error + config common.ServiceConfig + endpointStateStore store.KeyValueStore ) config.Version = version @@ -541,9 +542,27 @@ func main() { logger.Errorf("Failed to start nmagent client due to error %v", err) return } + + // Initialize endpoint state store if cns is managing endpoint state. + if cnsconfig.ManageEndpointState { + lockclient, err = processlock.NewFileLock(platform.CNILockPath + "endpoints" + store.LockExtension) + if err != nil { + log.Printf("Error initializing endpoint state file lock:%v", err) + return + } + + // Create the key value store. + storeFileName := storeFileLocation + "endpoints" + ".json" + endpointStateStore, err = store.NewJsonFileStore(storeFileName, lockclient) + if err != nil { + logger.Errorf("Failed to create endpoint state store file: %s, due to error %v\n", storeFileName, err) + return + } + } + // Create CNS object. - httpRestService, err := restserver.NewHTTPRestService(&config, &wireserver.Client{HTTPClient: &http.Client{}}, nmaclient) + httpRestService, err := restserver.NewHTTPRestService(&config, &wireserver.Client{HTTPClient: &http.Client{}}, nmaclient, endpointStateStore) if err != nil { logger.Errorf("Failed to create CNS object, err:%v.\n", err) return @@ -557,6 +576,7 @@ func main() { httpRestService.SetOption(acn.OptHttpConnectionTimeout, httpConnectionTimeout) httpRestService.SetOption(acn.OptHttpResponseHeaderTimeout, httpResponseHeaderTimeout) httpRestService.SetOption(acn.OptProgramSNATIPTables, cnsconfig.ProgramSNATIPTables) + httpRestService.SetOption(acn.OptManageEndpointState, cnsconfig.ManageEndpointState) // Create default ext network if commandline option is set if len(strings.TrimSpace(createDefaultExtNetworkType)) > 0 { @@ -973,7 +993,13 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn } var podInfoByIPProvider cns.PodInfoByIPProvider - if cnsconfig.InitializeFromCNI { + if cnsconfig.ManageEndpointState { + logger.Printf("Initializing from self managed endpoint store") + podInfoByIPProvider, err = cnireconciler.NewCNSPodInfoProvider(httpRestServiceImplementation.EndpointStateStore) // get reference to endpoint state store from rest server + if err != nil { + return errors.Wrap(err, "failed to create CNS PodInfoProvider") + } + } else if cnsconfig.InitializeFromCNI { logger.Printf("Initializing from CNI") podInfoByIPProvider, err = cnireconciler.NewCNIPodInfoProvider() if err != nil { diff --git a/cns/types/codes.go b/cns/types/codes.go index 6d75e13e1b..b61e1b3ba5 100644 --- a/cns/types/codes.go +++ b/cns/types/codes.go @@ -39,6 +39,7 @@ const ( NmAgentSupportedApisError ResponseCode = 37 UnsupportedNCVersion ResponseCode = 38 FailedToRunIPTableCmd ResponseCode = 39 + NilEndpointStateStore ResponseCode = 40 UnexpectedError ResponseCode = 99 ) diff --git a/common/config.go b/common/config.go index 996491828a..d3f17855f4 100644 --- a/common/config.go +++ b/common/config.go @@ -102,6 +102,9 @@ const ( OptHttpResponseHeaderTimeout = "http-response-header-timeout" OptHttpResponseHeaderTimeoutAlias = "httprespheadertimeout" + // Enable CNS to manage endpoint state + OptManageEndpointState = "manage-endpoint-state" + // Store file location OptStoreFileLocation = "store-file-path" OptStoreFileLocationAlias = "storefilepath" From cb599d6995a1605b6dcbf96ad44e94e3568be2e0 Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Mon, 1 Aug 2022 23:58:52 +0000 Subject: [PATCH 02/16] linting --- cns/service/main.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cns/service/main.go b/cns/service/main.go index e277a37016..462ae316de 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -993,19 +993,20 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn } var podInfoByIPProvider cns.PodInfoByIPProvider - if cnsconfig.ManageEndpointState { + switch { + case cnsconfig.ManageEndpointState: logger.Printf("Initializing from self managed endpoint store") podInfoByIPProvider, err = cnireconciler.NewCNSPodInfoProvider(httpRestServiceImplementation.EndpointStateStore) // get reference to endpoint state store from rest server if err != nil { return errors.Wrap(err, "failed to create CNS PodInfoProvider") } - } else if cnsconfig.InitializeFromCNI { + case cnsconfig.InitializeFromCNI: logger.Printf("Initializing from CNI") podInfoByIPProvider, err = cnireconciler.NewCNIPodInfoProvider() if err != nil { return errors.Wrap(err, "failed to create CNI PodInfoProvider") } - } else { + default: logger.Printf("Initializing from Kubernetes") podInfoByIPProvider = cns.PodInfoByIPProviderFunc(func() (map[string]cns.PodInfo, error) { pods, err := clientset.CoreV1().Pods("").List(ctx, metav1.ListOptions{ //nolint:govet // ignore err shadow @@ -1021,7 +1022,6 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn return podInfo, nil }) } - // create scoped kube clients. nnccli, err := nodenetworkconfig.NewClient(kubeConfig) if err != nil { From 1a41ab698e56fc3b0eace47c4bfeb63725cf577f Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Mon, 15 Aug 2022 20:17:19 +0000 Subject: [PATCH 03/16] rebase --- cns/cnireconciler/podinfoprovider.go | 21 +++++++--- cns/restserver/ipam.go | 57 +++++++++++++++++++++------- cns/restserver/restserver.go | 11 +++++- cns/restserver/util.go | 18 +++++++++ 4 files changed, 87 insertions(+), 20 deletions(-) diff --git a/cns/cnireconciler/podinfoprovider.go b/cns/cnireconciler/podinfoprovider.go index 91d749e62e..b5a69908f2 100644 --- a/cns/cnireconciler/podinfoprovider.go +++ b/cns/cnireconciler/podinfoprovider.go @@ -65,16 +65,27 @@ func cniStateToPodInfoByIP(state *api.AzureCNIState) (map[string]cns.PodInfo, er func endpointStateToPodInfoByIP(state map[string]*restserver.EndpointInfo) (map[string]cns.PodInfo, error) { podInfoByIP := map[string]cns.PodInfo{} for containerID, endpointInfo := range state { - for _, ipnet := range endpointInfo.IfnameToIPMap { - if _, ok := podInfoByIP[ipnet.IP.String()]; ok { - return nil, errors.Wrap(cns.ErrDuplicateIP, ipnet.IP.String()) + for ifname, ipinfo := range endpointInfo.IfnameToIPMap { + if _, ok := podInfoByIP[ipinfo.IPv4.IP.String()]; ok { + return nil, errors.Wrap(cns.ErrDuplicateIP, ipinfo.IPv4.IP.String()) } - podInfoByIP[ipnet.IP.String()] = cns.NewPodInfo( + podInfoByIP[ipinfo.IPv4.IP.String()] = cns.NewPodInfo( containerID, - containerID, // decided to treated podInterfaceID and podContainerID to be the same + ifname, endpointInfo.PodName, endpointInfo.PodNamespace, ) + if ipinfo.IPv6 != nil { // check for IPv6 + if _, ok := podInfoByIP[ipinfo.IPv6.IP.String()]; ok { + return nil, errors.Wrap(cns.ErrDuplicateIP, ipinfo.IPv6.IP.String()) + } + podInfoByIP[ipinfo.IPv6.IP.String()] = cns.NewPodInfo( + containerID, + ifname, + endpointInfo.PodName, + endpointInfo.PodNamespace, + ) + } } } return podInfoByIP, nil diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index e0a6455659..3d260fb012 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -103,29 +103,33 @@ func (service *HTTPRestService) updateEndpointState(ipconfigRequest cns.IPConfig } service.Lock() defer service.Unlock() - if service.EndpointState == nil { // if endpoint state is empty then read from store - err := service.EndpointStateStore.Read("endpoints", service.EndpointState) - if err != nil { - return fmt.Errorf("failed to read endpoint state from store: %w", err) - } - } - - if endpointInfo, ok := service.EndpointState[ipconfigRequest.InfraContainerID]; ok { + logger.Printf("[updateEndpointState] Updating endpoint state for infra container %s", ipconfigRequest.InfraContainerID) + if endpointInfo, ok := service.EndpointState.ContainerInterfaces[ipconfigRequest.InfraContainerID]; ok { + logger.Printf("[updateEndpointState] Found existing endpoint state for infra container %s", ipconfigRequest.InfraContainerID) _, ipnet, err := net.ParseCIDR(podIPInfo.PodIPConfig.IPAddress + "/" + fmt.Sprint(podIPInfo.PodIPConfig.PrefixLength)) if err != nil { return fmt.Errorf("failed to parse pod ip address: %w", err) } - endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname] = ipnet - service.EndpointState[ipconfigRequest.InfraContainerID] = endpointInfo + if ipnet.IP.To4() == nil { // is an ipv6 address + endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv6 = ipnet + } else { + endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv4 = ipnet + } + + service.EndpointState.ContainerInterfaces[ipconfigRequest.InfraContainerID] = endpointInfo } else { - endpointInfo := &EndpointInfo{PodName: podInfo.Name(), PodNamespace: podInfo.Namespace(), IfnameToIPMap: make(map[string]*net.IPNet)} + endpointInfo := &EndpointInfo{PodName: podInfo.Name(), PodNamespace: podInfo.Namespace(), IfnameToIPMap: make(map[string]*IPInfo)} _, ipnet, err := net.ParseCIDR(podIPInfo.PodIPConfig.IPAddress + "/" + fmt.Sprint(podIPInfo.PodIPConfig.PrefixLength)) if err != nil { return fmt.Errorf("failed to parse pod ip address: %w", err) } - endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname] = ipnet - service.EndpointState[ipconfigRequest.InfraContainerID] = endpointInfo + if ipnet.IP.To4() == nil { // is an ipv6 address + endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv6 = ipnet + } else { + endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv4 = ipnet + } + service.EndpointState.ContainerInterfaces[ipconfigRequest.InfraContainerID] = endpointInfo } err := service.EndpointStateStore.Write("endpoints", service.EndpointState) @@ -153,6 +157,18 @@ func (service *HTTPRestService) releaseIPConfigHandler(w http.ResponseWriter, r podInfo, returnCode, message := service.validateIPConfigRequest(req) + if err = service.removeEndpointState(podInfo); err != nil { + resp := cns.Response{ + ReturnCode: types.UnexpectedError, + Message: err.Error(), + } + logger.Errorf("releaseIPConfigHandler remove endpoint state failed because %v, release IP config info %s", resp.Message, req) + w.Header().Set(cnsReturnCode, resp.ReturnCode.String()) + err = service.Listener.Encode(w, &resp) + logger.ResponseEx(service.Name, req, resp, resp.ReturnCode, err) + return + } + if err = service.releaseIPConfig(podInfo); err != nil { returnCode = types.UnexpectedError message = err.Error() @@ -167,6 +183,21 @@ func (service *HTTPRestService) releaseIPConfigHandler(w http.ResponseWriter, r logger.ResponseEx(service.Name, req, resp, resp.ReturnCode, err) } +func (service *HTTPRestService) removeEndpointState(podInfo cns.PodInfo) error { + if service.EndpointStateStore == nil { + return fmt.Errorf("nil endpoint state store") + } + service.Lock() + defer service.Unlock() + logger.Printf("[removeEndpointState] Removing endpoint state for infra container %s", podInfo.InfraContainerID()) + if _, ok := service.EndpointState.ContainerInterfaces[podInfo.InfraContainerID()]; ok { + delete(service.EndpointState.ContainerInterfaces, podInfo.InfraContainerID()) + } else { // will not fail if no endpoint state for infra container id is found + logger.Printf("[removeEndpointState] No endpoint state found for infra container %s", podInfo.InfraContainerID()) + } + return nil +} + // MarkIPAsPendingRelease will set the IPs which are in PendingProgramming or Available to PendingRelease state // It will try to update [totalIpsToRelease] number of ips. func (service *HTTPRestService) MarkIPAsPendingRelease(totalIpsToRelease int) (map[string]cns.IPConfigurationStatus, error) { diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index 30bea98d8a..6d04819bff 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -60,14 +60,20 @@ type HTTPRestService struct { podsPendingIPAssignment *bounded.TimedSet sync.RWMutex dncPartitionKey string - EndpointState map[string]*EndpointInfo // key : container ID + programmedIPtables bool + EndpointState map[string]*EndpointInfo // key : container id EndpointStateStore store.KeyValueStore } type EndpointInfo struct { PodName string PodNamespace string - IfnameToIPMap map[string]*net.IPNet + IfnameToIPMap map[string]*IPInfo // key : interface name, value : IPInfo +} + +type IPInfo struct { + IPv4 *net.IPNet + IPv6 *net.IPNet } type GetHTTPServiceDataResponse struct { @@ -168,6 +174,7 @@ func NewHTTPRestService(config *common.ServiceConfig, wscli interfaceGetter, nma state: serviceState, podsPendingIPAssignment: bounded.NewTimedSet(250), // nolint:gomnd // maxpods EndpointStateStore: endpointStateStore, + EndpointState: make(map[string]*EndpointInfo), }, nil } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 8696125019..7e07502278 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -17,6 +17,7 @@ import ( "github.com/Azure/azure-container-networking/cns/nmagent" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/cns/wireserver" + "github.com/Azure/azure-container-networking/common" acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/platform" "github.com/Azure/azure-container-networking/store" @@ -95,6 +96,23 @@ func (service *HTTPRestService) restoreState() { } logger.Printf("[Azure CNS] Restored state, %+v\n", service.state) + + if service.Options[common.OptManageEndpointState] == true { + err := service.EndpointStateStore.Read("endpoints", &service.EndpointState) + if err != nil { + if err == store.ErrKeyNotFound { + // Nothing to restore. + logger.Printf("[Azure CNS] No endpoint state to restore.\n") + } else { + logger.Errorf("[Azure CNS] Failed to restore endpoint state, err:%v. Removing endpoints.json", err) + service.EndpointStateStore.Remove() + } + return + } + logger.Printf("[Azure CNS] Restored endpoint state, %+v\n", service.EndpointState) + + } + } func (service *HTTPRestService) saveNetworkContainerGoalState( From 69e12c9fcb91a8463638b07423e8cd403a35a803 Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Wed, 3 Aug 2022 07:06:28 +0000 Subject: [PATCH 04/16] missing if condition for releaseIPConfig --- cns/restserver/ipam.go | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 3d260fb012..3cf5b70646 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -157,16 +157,19 @@ func (service *HTTPRestService) releaseIPConfigHandler(w http.ResponseWriter, r podInfo, returnCode, message := service.validateIPConfigRequest(req) - if err = service.removeEndpointState(podInfo); err != nil { - resp := cns.Response{ - ReturnCode: types.UnexpectedError, - Message: err.Error(), + // Check if http rest service managed endpoint state is set + if service.Options[common.OptManageEndpointState] == true { + if err = service.removeEndpointState(podInfo); err != nil { + resp := cns.Response{ + ReturnCode: types.UnexpectedError, + Message: err.Error(), + } + logger.Errorf("releaseIPConfigHandler remove endpoint state failed because %v, release IP config info %s", resp.Message, req) + w.Header().Set(cnsReturnCode, resp.ReturnCode.String()) + err = service.Listener.Encode(w, &resp) + logger.ResponseEx(service.Name, req, resp, resp.ReturnCode, err) + return } - logger.Errorf("releaseIPConfigHandler remove endpoint state failed because %v, release IP config info %s", resp.Message, req) - w.Header().Set(cnsReturnCode, resp.ReturnCode.String()) - err = service.Listener.Encode(w, &resp) - logger.ResponseEx(service.Name, req, resp, resp.ReturnCode, err) - return } if err = service.releaseIPConfig(podInfo); err != nil { From cd7b02fbf617db424d91bec26d2d689029e237a3 Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Tue, 9 Aug 2022 00:46:12 +0000 Subject: [PATCH 05/16] update azure-cns.yaml and add UTs --- cns/azure-cns.yaml | 11 +++- cns/cnireconciler/podinfoprovider_test.go | 46 +++++++++++++++ cns/restserver/ipam_test.go | 69 ++++++++++++++++++++++- store/mockstore.go | 20 ++++++- 4 files changed, 143 insertions(+), 3 deletions(-) diff --git a/cns/azure-cns.yaml b/cns/azure-cns.yaml index 9bb98b713e..541c2a6d84 100644 --- a/cns/azure-cns.yaml +++ b/cns/azure-cns.yaml @@ -104,6 +104,8 @@ spec: mountPath: /var/log - name: cns-state mountPath: /var/lib/azure-network + - name: azure-endpoints + mountPath: /var/run/azure-cns/ - name: cns-config mountPath: /etc/azure-cns - name: cni-bin @@ -130,6 +132,10 @@ spec: fieldPath: spec.nodeName hostNetwork: true volumes: + - name: azure-endpoints + hostPath: + path: /var/run/azure-cns/ + type: DirectoryOrCreate - name: log hostPath: path: /var/log @@ -179,5 +185,8 @@ data: "NodeSyncIntervalInSeconds": 30 }, "ChannelMode": "CRD", - "InitializeFromCNI": true + "InitializeFromCNI": true, + "ManageEndpointState": false, + "ProgramSNATIPTables" : false } +# Toggle ManageEndpointState and ProgramSNATIPTables to true for delegated IPAM use case. diff --git a/cns/cnireconciler/podinfoprovider_test.go b/cns/cnireconciler/podinfoprovider_test.go index 8c426a3630..7187644de2 100644 --- a/cns/cnireconciler/podinfoprovider_test.go +++ b/cns/cnireconciler/podinfoprovider_test.go @@ -1,9 +1,12 @@ package cnireconciler import ( + "net" "testing" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/restserver" + "github.com/Azure/azure-container-networking/store" testutils "github.com/Azure/azure-container-networking/test/utils" "github.com/stretchr/testify/assert" "k8s.io/utils/exec" @@ -59,3 +62,46 @@ func TestNewCNIPodInfoProvider(t *testing.T) { }) } } + +func TestNewCNSPodInfoProvider(t *testing.T) { + goodStore := store.NewMockStore("") + goodEndpointState := make(map[string]*restserver.EndpointInfo) + endpointInfo := &restserver.EndpointInfo{PodName: "goldpinger-deploy-bbbf9fd7c-z8v4l", PodNamespace: "default", IfnameToIPMap: make(map[string]*restserver.IPInfo)} + endpointInfo.IfnameToIPMap["eth0"] = &restserver.IPInfo{IPv4: &net.IPNet{IP: net.IPv4(10, 241, 0, 65), Mask: net.IPv4Mask(255, 255, 255, 0)}} + + goodEndpointState["0a4917617e15d24dc495e407d8eb5c88e4406e58fa209e4eb75a2c2fb7045eea"] = endpointInfo + _ = goodStore.Write(restserver.EndpointStoreKey, goodEndpointState) + tests := []struct { + name string + store store.KeyValueStore + want map[string]cns.PodInfo + wantErr bool + }{ + { + name: "good", + store: goodStore, + want: map[string]cns.PodInfo{"10.241.0.65": cns.NewPodInfo("0a4917617e15d24dc495e407d8eb5c88e4406e58fa209e4eb75a2c2fb7045eea", "eth0", "goldpinger-deploy-bbbf9fd7c-z8v4l", "default")}, + wantErr: false, + }, + { + name: "empty store", + store: store.NewMockStore(""), + want: map[string]cns.PodInfo{}, + wantErr: true, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + got, err := newCNSPodInfoProvider(tt.store) + if tt.wantErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + podInfoByIP, _ := got.PodInfoByIP() + assert.Equal(t, tt.want, podInfoByIP) + }) + } +} diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 35e77e68a0..04e4f27dfa 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -4,6 +4,8 @@ package restserver import ( + "fmt" + "net" "strconv" "testing" @@ -12,6 +14,7 @@ import ( "github.com/Azure/azure-container-networking/cns/fakes" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" + "github.com/Azure/azure-container-networking/store" "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) @@ -37,7 +40,7 @@ var ( func getTestService() *HTTPRestService { var config common.ServiceConfig - httpsvc, _ := NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.NMAgentClientFake{}, nil) + httpsvc, _ := NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.NMAgentClientFake{}, store.NewMockStore("")) svc = httpsvc.(*HTTPRestService) svc.IPAMPoolMonitor = &fakes.MonitorFake{} setOrchestratorTypeInternal(cns.KubernetesCRD) @@ -125,6 +128,70 @@ func UpdatePodIpConfigState(t *testing.T, svc *HTTPRestService, ipconfigs map[st return nil } +func TestEndpointStateReadAndWrite(t *testing.T) { + svc := getTestService() + testState := NewPodState(testIP1, 24, testPod1GUID, testNCID, types.Available, 0) + ipconfigs := map[string]cns.IPConfigurationStatus{ + testState.ID: testState, + } + err := UpdatePodIpConfigState(t, svc, ipconfigs) + if err != nil { + t.Fatalf("Expected to not fail update service with config: %+v", err) + } + req := cns.IPConfigRequest{ + PodInterfaceID: testPod1Info.InterfaceID(), + InfraContainerID: testPod1Info.InfraContainerID(), + } + b, _ := testPod1Info.OrchestratorContext() + req.OrchestratorContext = b + req.Ifname = "eth0" + podIPInfo, err := requestIPConfigHelper(svc, req) + if err != nil { + t.Fatalf("Expected to not fail getting pod ip info: %+v", err) + } + ip, ipnet, err := net.ParseCIDR(podIPInfo.PodIPConfig.IPAddress + "/" + fmt.Sprint(podIPInfo.PodIPConfig.PrefixLength)) + if err != nil { + t.Fatalf("failed to parse pod ip address: %+v", err) + } + ipconfig := &net.IPNet{IP: ip, Mask: ipnet.Mask} + ipInfo := &IPInfo{} + if ip.To4() == nil { // is an ipv6 address + ipInfo.IPv6 = ipconfig + } else { + ipInfo.IPv4 = ipconfig + } + + // add + desiredState := map[string]*EndpointInfo{req.InfraContainerID: {PodName: testPod1Info.Name(), PodNamespace: testPod1Info.Namespace(), IfnameToIPMap: map[string]*IPInfo{req.Ifname: ipInfo}}} + err = svc.updateEndpointState(req, testPod1Info, podIPInfo) + if err != nil { + t.Fatalf("Expected to not fail updating endpoint state: %+v", err) + } + assert.Equal(t, desiredState, svc.EndpointState) + + // consecutive add of same endpoint should not change state or cause error + err = svc.updateEndpointState(req, testPod1Info, podIPInfo) + if err != nil { + t.Fatalf("Expected to not fail updating existing endpoint state: %+v", err) + } + assert.Equal(t, desiredState, svc.EndpointState) + + // delete + desiredState = map[string]*EndpointInfo{} + err = svc.removeEndpointState(testPod1Info) + if err != nil { + t.Fatalf("Expected to not fail removing endpoint state: %+v", err) + } + assert.Equal(t, desiredState, svc.EndpointState) + + // delete non-existent endpoint should not change state or cause error + err = svc.removeEndpointState(testPod1Info) + if err != nil { + t.Fatalf("Expected to not fail removing non existing key: %+v", err) + } + assert.Equal(t, desiredState, svc.EndpointState) +} + // Want first IP func TestIPAMGetAvailableIPConfig(t *testing.T) { svc := getTestService() diff --git a/store/mockstore.go b/store/mockstore.go index 4077510d6b..7dda8b1938 100644 --- a/store/mockstore.go +++ b/store/mockstore.go @@ -1,30 +1,48 @@ package store import ( + "encoding/json" + "fmt" "time" ) type mockStore struct { lockFilePath string + data map[string]*json.RawMessage } // NewMockStore creates a new jsonFileStore object, accessed as a KeyValueStore. func NewMockStore(lockFilePath string) KeyValueStore { return &mockStore{ lockFilePath: lockFilePath, + data: make(map[string]*json.RawMessage), } } func (ms *mockStore) Exists() bool { - return false + return ms.data == nil } // Read restores the value for the given key from persistent store. func (ms *mockStore) Read(key string, value interface{}) error { + if _, ok := ms.data[key]; !ok { + return ErrStoreEmpty + } + err := json.Unmarshal(*ms.data[key], value) + if err != nil { + return fmt.Errorf("%w", err) + } return nil } func (ms *mockStore) Write(key string, value interface{}) error { + var raw json.RawMessage + raw, err := json.Marshal(value) + if err != nil { + return fmt.Errorf("%w", err) + } + + ms.data[key] = &raw return nil } From d04352feff3a14a38c0ffd894849e5f1cf261484 Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Mon, 15 Aug 2022 20:20:31 +0000 Subject: [PATCH 06/16] rebase --- cns/restserver/ipam.go | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 3cf5b70646..1273cbcfc5 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -97,42 +97,48 @@ func (service *HTTPRestService) requestIPConfigHandler(w http.ResponseWriter, r logger.ResponseEx(service.Name+operationName, ipconfigRequest, reserveResp, reserveResp.Response.ReturnCode, err) } +var errNilStateStore = errors.New("nil endpoint state store") + func (service *HTTPRestService) updateEndpointState(ipconfigRequest cns.IPConfigRequest, podInfo cns.PodInfo, podIPInfo cns.PodIpInfo) error { if service.EndpointStateStore == nil { - return fmt.Errorf("nil endpoint state store") + return errNilStateStore } service.Lock() defer service.Unlock() logger.Printf("[updateEndpointState] Updating endpoint state for infra container %s", ipconfigRequest.InfraContainerID) - if endpointInfo, ok := service.EndpointState.ContainerInterfaces[ipconfigRequest.InfraContainerID]; ok { + if endpointInfo, ok := service.EndpointState[ipconfigRequest.InfraContainerID]; ok { logger.Printf("[updateEndpointState] Found existing endpoint state for infra container %s", ipconfigRequest.InfraContainerID) - _, ipnet, err := net.ParseCIDR(podIPInfo.PodIPConfig.IPAddress + "/" + fmt.Sprint(podIPInfo.PodIPConfig.PrefixLength)) + ip, ipnet, err := net.ParseCIDR(podIPInfo.PodIPConfig.IPAddress + "/" + fmt.Sprint(podIPInfo.PodIPConfig.PrefixLength)) if err != nil { return fmt.Errorf("failed to parse pod ip address: %w", err) } - if ipnet.IP.To4() == nil { // is an ipv6 address - endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv6 = ipnet + ipconfig := &net.IPNet{IP: ip, Mask: ipnet.Mask} + if ip.To4() == nil { // is an ipv6 address + endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv6 = ipconfig } else { - endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv4 = ipnet + endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv4 = ipconfig } - service.EndpointState.ContainerInterfaces[ipconfigRequest.InfraContainerID] = endpointInfo + service.EndpointState[ipconfigRequest.InfraContainerID] = endpointInfo } else { endpointInfo := &EndpointInfo{PodName: podInfo.Name(), PodNamespace: podInfo.Namespace(), IfnameToIPMap: make(map[string]*IPInfo)} - _, ipnet, err := net.ParseCIDR(podIPInfo.PodIPConfig.IPAddress + "/" + fmt.Sprint(podIPInfo.PodIPConfig.PrefixLength)) + ip, ipnet, err := net.ParseCIDR(podIPInfo.PodIPConfig.IPAddress + "/" + fmt.Sprint(podIPInfo.PodIPConfig.PrefixLength)) if err != nil { return fmt.Errorf("failed to parse pod ip address: %w", err) } - if ipnet.IP.To4() == nil { // is an ipv6 address - endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv6 = ipnet + ipconfig := &net.IPNet{IP: ip, Mask: ipnet.Mask} + ipInfo := &IPInfo{} + if ip.To4() == nil { // is an ipv6 address + ipInfo.IPv6 = ipconfig } else { - endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv4 = ipnet + ipInfo.IPv4 = ipconfig } - service.EndpointState.ContainerInterfaces[ipconfigRequest.InfraContainerID] = endpointInfo + endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname] = ipInfo + service.EndpointState[ipconfigRequest.InfraContainerID] = endpointInfo } - err := service.EndpointStateStore.Write("endpoints", service.EndpointState) + err := service.EndpointStateStore.Write(EndpointStoreKey, service.EndpointState) if err != nil { return fmt.Errorf("failed to write endpoint state to store: %w", err) } @@ -188,13 +194,17 @@ func (service *HTTPRestService) releaseIPConfigHandler(w http.ResponseWriter, r func (service *HTTPRestService) removeEndpointState(podInfo cns.PodInfo) error { if service.EndpointStateStore == nil { - return fmt.Errorf("nil endpoint state store") + return errNilStateStore } service.Lock() defer service.Unlock() logger.Printf("[removeEndpointState] Removing endpoint state for infra container %s", podInfo.InfraContainerID()) - if _, ok := service.EndpointState.ContainerInterfaces[podInfo.InfraContainerID()]; ok { - delete(service.EndpointState.ContainerInterfaces, podInfo.InfraContainerID()) + if _, ok := service.EndpointState[podInfo.InfraContainerID()]; ok { + delete(service.EndpointState, podInfo.InfraContainerID()) + err := service.EndpointStateStore.Write(EndpointStoreKey, service.EndpointState) + if err != nil { + return fmt.Errorf("failed to write endpoint state to store: %w", err) + } } else { // will not fail if no endpoint state for infra container id is found logger.Printf("[removeEndpointState] No endpoint state found for infra container %s", podInfo.InfraContainerID()) } From 3bcf1d33993cc222abf4d767abd9aeaec03150af Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Mon, 15 Aug 2022 20:27:16 +0000 Subject: [PATCH 07/16] update program iptables changes --- cns/cnireconciler/podinfoprovider.go | 2 +- cns/restserver/const.go | 7 ++++--- cns/restserver/restserver.go | 1 - cns/restserver/util.go | 2 +- cns/service/main.go | 13 ++++++++++--- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/cns/cnireconciler/podinfoprovider.go b/cns/cnireconciler/podinfoprovider.go index b5a69908f2..ae26315d4c 100644 --- a/cns/cnireconciler/podinfoprovider.go +++ b/cns/cnireconciler/podinfoprovider.go @@ -24,7 +24,7 @@ func NewCNSPodInfoProvider(endpointStore store.KeyValueStore) (cns.PodInfoByIPPr func newCNSPodInfoProvider(endpointStore store.KeyValueStore) (cns.PodInfoByIPProvider, error) { var state map[string]*restserver.EndpointInfo - err := endpointStore.Read("endpoints", state) + err := endpointStore.Read(restserver.EndpointStoreKey, state) if err != nil { return nil, fmt.Errorf("failed to read endpoints state from store : %w", err) } diff --git a/cns/restserver/const.go b/cns/restserver/const.go index 9d94a74fa8..d51814a1d7 100644 --- a/cns/restserver/const.go +++ b/cns/restserver/const.go @@ -2,9 +2,10 @@ package restserver const ( // Key against which CNS state is persisted. - storeKey = "ContainerNetworkService" - attach = "Attach" - detach = "Detach" + storeKey = "ContainerNetworkService" + EndpointStoreKey = "Endpoints" + attach = "Attach" + detach = "Detach" // Rest service state identifier for named lock stateJoinedNetworks = "JoinedNetworks" dncApiVersion = "?api-version=2018-03-01" diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index 6d04819bff..4ff57f17ed 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -60,7 +60,6 @@ type HTTPRestService struct { podsPendingIPAssignment *bounded.TimedSet sync.RWMutex dncPartitionKey string - programmedIPtables bool EndpointState map[string]*EndpointInfo // key : container id EndpointStateStore store.KeyValueStore } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 7e07502278..0c688cbcb7 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -98,7 +98,7 @@ func (service *HTTPRestService) restoreState() { logger.Printf("[Azure CNS] Restored state, %+v\n", service.state) if service.Options[common.OptManageEndpointState] == true { - err := service.EndpointStateStore.Read("endpoints", &service.EndpointState) + err := service.EndpointStateStore.Read(EndpointStoreKey, &service.EndpointState) if err != nil { if err == store.ErrKeyNotFound { // Nothing to restore. diff --git a/cns/service/main.go b/cns/service/main.go index 462ae316de..3a434a175c 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -67,6 +67,8 @@ const ( // Service name. name = "azure-cns" pluginName = "azure-vnet" + endpointStoreName = "azure-endpoints" + endpointStoreLocation = "/var/run/azure-cns/" defaultCNINetworkConfigFileName = "10-azure.conflist" dncApiVersion = "?api-version=2018-03-01" poolIPAMRefreshRateInMilliseconds = 1000 @@ -545,14 +547,19 @@ func main() { // Initialize endpoint state store if cns is managing endpoint state. if cnsconfig.ManageEndpointState { - lockclient, err = processlock.NewFileLock(platform.CNILockPath + "endpoints" + store.LockExtension) + log.Printf("[Azure CNS] Configured to manage endpoints state") + lockclient, err = processlock.NewFileLock(platform.CNILockPath + endpointStoreName + store.LockExtension) if err != nil { log.Printf("Error initializing endpoint state file lock:%v", err) return } - + err = platform.CreateDirectory(endpointStoreLocation) + if err != nil { + logger.Errorf("Failed to create File Store directory %s, due to Error:%v", storeFileLocation, err.Error()) + return + } // Create the key value store. - storeFileName := storeFileLocation + "endpoints" + ".json" + storeFileName := endpointStoreLocation + endpointStoreName + ".json" endpointStateStore, err = store.NewJsonFileStore(storeFileName, lockclient) if err != nil { logger.Errorf("Failed to create endpoint state store file: %s, due to error %v\n", storeFileName, err) From ddc95430c0443a2526aad42b35ce5f8259d828e7 Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Mon, 15 Aug 2022 20:47:42 +0000 Subject: [PATCH 08/16] linting --- cns/restserver/util.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 0c688cbcb7..2bb44956e6 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -17,7 +17,6 @@ import ( "github.com/Azure/azure-container-networking/cns/nmagent" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/cns/wireserver" - "github.com/Azure/azure-container-networking/common" acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/platform" "github.com/Azure/azure-container-networking/store" @@ -97,10 +96,10 @@ func (service *HTTPRestService) restoreState() { logger.Printf("[Azure CNS] Restored state, %+v\n", service.state) - if service.Options[common.OptManageEndpointState] == true { + if service.Options[acn.OptManageEndpointState] == true { err := service.EndpointStateStore.Read(EndpointStoreKey, &service.EndpointState) if err != nil { - if err == store.ErrKeyNotFound { + if errors.Is(err, store.ErrKeyNotFound) { // Nothing to restore. logger.Printf("[Azure CNS] No endpoint state to restore.\n") } else { @@ -112,7 +111,6 @@ func (service *HTTPRestService) restoreState() { logger.Printf("[Azure CNS] Restored endpoint state, %+v\n", service.EndpointState) } - } func (service *HTTPRestService) saveNetworkContainerGoalState( From 46cb82fadd19b90dbbf1e621b19c9ebec5b3ce3a Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Mon, 15 Aug 2022 21:43:34 +0000 Subject: [PATCH 09/16] fix broken tests --- cns/cnireconciler/podinfoprovider.go | 2 +- cns/cnireconciler/podinfoprovider_test.go | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cns/cnireconciler/podinfoprovider.go b/cns/cnireconciler/podinfoprovider.go index ae26315d4c..44b8abc1b5 100644 --- a/cns/cnireconciler/podinfoprovider.go +++ b/cns/cnireconciler/podinfoprovider.go @@ -24,7 +24,7 @@ func NewCNSPodInfoProvider(endpointStore store.KeyValueStore) (cns.PodInfoByIPPr func newCNSPodInfoProvider(endpointStore store.KeyValueStore) (cns.PodInfoByIPProvider, error) { var state map[string]*restserver.EndpointInfo - err := endpointStore.Read(restserver.EndpointStoreKey, state) + err := endpointStore.Read(restserver.EndpointStoreKey, &state) if err != nil { return nil, fmt.Errorf("failed to read endpoints state from store : %w", err) } diff --git a/cns/cnireconciler/podinfoprovider_test.go b/cns/cnireconciler/podinfoprovider_test.go index 7187644de2..0efa6cd714 100644 --- a/cns/cnireconciler/podinfoprovider_test.go +++ b/cns/cnireconciler/podinfoprovider_test.go @@ -70,7 +70,10 @@ func TestNewCNSPodInfoProvider(t *testing.T) { endpointInfo.IfnameToIPMap["eth0"] = &restserver.IPInfo{IPv4: &net.IPNet{IP: net.IPv4(10, 241, 0, 65), Mask: net.IPv4Mask(255, 255, 255, 0)}} goodEndpointState["0a4917617e15d24dc495e407d8eb5c88e4406e58fa209e4eb75a2c2fb7045eea"] = endpointInfo - _ = goodStore.Write(restserver.EndpointStoreKey, goodEndpointState) + err := goodStore.Write(restserver.EndpointStoreKey, goodEndpointState) + if err != nil { + t.Fatalf("Error writing to store: %v", err) + } tests := []struct { name string store store.KeyValueStore From a3f428c3d25daed772997a7b543a1577c33dd6d9 Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Mon, 15 Aug 2022 15:07:20 -0700 Subject: [PATCH 10/16] fix podinfoprovider returns error when key is not found --- cns/cnireconciler/podinfoprovider.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cns/cnireconciler/podinfoprovider.go b/cns/cnireconciler/podinfoprovider.go index 44b8abc1b5..cda99c2abe 100644 --- a/cns/cnireconciler/podinfoprovider.go +++ b/cns/cnireconciler/podinfoprovider.go @@ -26,6 +26,12 @@ func newCNSPodInfoProvider(endpointStore store.KeyValueStore) (cns.PodInfoByIPPr var state map[string]*restserver.EndpointInfo err := endpointStore.Read(restserver.EndpointStoreKey, &state) if err != nil { + if errors.Is(err, store.ErrKeyNotFound) { + // Nothing to restore. + return cns.PodInfoByIPProviderFunc(func() (map[string]cns.PodInfo, error) { + return endpointStateToPodInfoByIP(state) + }), nil + } return nil, fmt.Errorf("failed to read endpoints state from store : %w", err) } return cns.PodInfoByIPProviderFunc(func() (map[string]cns.PodInfo, error) { From b195c2b6f66520fc2e788ada99e5744144eac1b0 Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Mon, 15 Aug 2022 15:27:37 -0700 Subject: [PATCH 11/16] log when no endpoint state exist when reconcilling --- cns/cnireconciler/podinfoprovider.go | 2 +- cns/service/main.go | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cns/cnireconciler/podinfoprovider.go b/cns/cnireconciler/podinfoprovider.go index cda99c2abe..4abf5895f1 100644 --- a/cns/cnireconciler/podinfoprovider.go +++ b/cns/cnireconciler/podinfoprovider.go @@ -30,7 +30,7 @@ func newCNSPodInfoProvider(endpointStore store.KeyValueStore) (cns.PodInfoByIPPr // Nothing to restore. return cns.PodInfoByIPProviderFunc(func() (map[string]cns.PodInfo, error) { return endpointStateToPodInfoByIP(state) - }), nil + }), err } return nil, fmt.Errorf("failed to read endpoints state from store : %w", err) } diff --git a/cns/service/main.go b/cns/service/main.go index 3a434a175c..36439e672c 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -1005,7 +1005,11 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn logger.Printf("Initializing from self managed endpoint store") podInfoByIPProvider, err = cnireconciler.NewCNSPodInfoProvider(httpRestServiceImplementation.EndpointStateStore) // get reference to endpoint state store from rest server if err != nil { - return errors.Wrap(err, "failed to create CNS PodInfoProvider") + if errors.Is(err, store.ErrKeyNotFound) { + logger.Printf("[Azure CNS] No endpoint state found, skipping initializing CNS state") + } else { + return errors.Wrap(err, "failed to create CNS PodInfoProvider") + } } case cnsconfig.InitializeFromCNI: logger.Printf("Initializing from CNI") From b427464507caef205ff08bcfd1b68b44065dd270 Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Mon, 15 Aug 2022 16:16:48 -0700 Subject: [PATCH 12/16] not remove endpoint state file on failure to read in restserver.restoreState() --- cns/restserver/util.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 2bb44956e6..05acd5701e 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -104,7 +104,6 @@ func (service *HTTPRestService) restoreState() { logger.Printf("[Azure CNS] No endpoint state to restore.\n") } else { logger.Errorf("[Azure CNS] Failed to restore endpoint state, err:%v. Removing endpoints.json", err) - service.EndpointStateStore.Remove() } return } From 4b852e9a73eeddcfc6b9143846ace978f964ecbd Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Tue, 16 Aug 2022 17:32:01 -0700 Subject: [PATCH 13/16] addressed comments --- cns/cnireconciler/podinfoprovider.go | 30 ++++++++++++----------- cns/cnireconciler/podinfoprovider_test.go | 2 +- cns/restserver/ipam.go | 30 ++++++++++++++++------- cns/restserver/ipam_test.go | 6 ++--- cns/restserver/restserver.go | 4 +-- 5 files changed, 43 insertions(+), 29 deletions(-) diff --git a/cns/cnireconciler/podinfoprovider.go b/cns/cnireconciler/podinfoprovider.go index 4abf5895f1..ee9e71a29d 100644 --- a/cns/cnireconciler/podinfoprovider.go +++ b/cns/cnireconciler/podinfoprovider.go @@ -70,22 +70,24 @@ func cniStateToPodInfoByIP(state *api.AzureCNIState) (map[string]cns.PodInfo, er func endpointStateToPodInfoByIP(state map[string]*restserver.EndpointInfo) (map[string]cns.PodInfo, error) { podInfoByIP := map[string]cns.PodInfo{} - for containerID, endpointInfo := range state { - for ifname, ipinfo := range endpointInfo.IfnameToIPMap { - if _, ok := podInfoByIP[ipinfo.IPv4.IP.String()]; ok { - return nil, errors.Wrap(cns.ErrDuplicateIP, ipinfo.IPv4.IP.String()) + for containerID, endpointInfo := range state { // for each endpoint + for ifname, ipinfo := range endpointInfo.IfnameToIPMap { // for each IP info object of the endpoint's interfaces + for _, ipv4conf := range ipinfo.IPv4 { // for each IPv4 config of the endpoint's interfaces + if _, ok := podInfoByIP[ipv4conf.IP.String()]; ok { + return nil, errors.Wrap(cns.ErrDuplicateIP, ipv4conf.IP.String()) + } + podInfoByIP[ipv4conf.IP.String()] = cns.NewPodInfo( + containerID, + ifname, + endpointInfo.PodName, + endpointInfo.PodNamespace, + ) } - podInfoByIP[ipinfo.IPv4.IP.String()] = cns.NewPodInfo( - containerID, - ifname, - endpointInfo.PodName, - endpointInfo.PodNamespace, - ) - if ipinfo.IPv6 != nil { // check for IPv6 - if _, ok := podInfoByIP[ipinfo.IPv6.IP.String()]; ok { - return nil, errors.Wrap(cns.ErrDuplicateIP, ipinfo.IPv6.IP.String()) + for _, ipv6conf := range ipinfo.IPv6 { // for each IPv6 config of the endpoint's interfaces + if _, ok := podInfoByIP[ipv6conf.IP.String()]; ok { + return nil, errors.Wrap(cns.ErrDuplicateIP, ipv6conf.IP.String()) } - podInfoByIP[ipinfo.IPv6.IP.String()] = cns.NewPodInfo( + podInfoByIP[ipv6conf.IP.String()] = cns.NewPodInfo( containerID, ifname, endpointInfo.PodName, diff --git a/cns/cnireconciler/podinfoprovider_test.go b/cns/cnireconciler/podinfoprovider_test.go index 0efa6cd714..d81ff3e0b1 100644 --- a/cns/cnireconciler/podinfoprovider_test.go +++ b/cns/cnireconciler/podinfoprovider_test.go @@ -67,7 +67,7 @@ func TestNewCNSPodInfoProvider(t *testing.T) { goodStore := store.NewMockStore("") goodEndpointState := make(map[string]*restserver.EndpointInfo) endpointInfo := &restserver.EndpointInfo{PodName: "goldpinger-deploy-bbbf9fd7c-z8v4l", PodNamespace: "default", IfnameToIPMap: make(map[string]*restserver.IPInfo)} - endpointInfo.IfnameToIPMap["eth0"] = &restserver.IPInfo{IPv4: &net.IPNet{IP: net.IPv4(10, 241, 0, 65), Mask: net.IPv4Mask(255, 255, 255, 0)}} + endpointInfo.IfnameToIPMap["eth0"] = &restserver.IPInfo{IPv4: []net.IPNet{{IP: net.IPv4(10, 241, 0, 65), Mask: net.IPv4Mask(255, 255, 255, 0)}}} goodEndpointState["0a4917617e15d24dc495e407d8eb5c88e4406e58fa209e4eb75a2c2fb7045eea"] = endpointInfo err := goodStore.Write(restserver.EndpointStoreKey, goodEndpointState) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 1273cbcfc5..a8abac332e 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -97,11 +97,11 @@ func (service *HTTPRestService) requestIPConfigHandler(w http.ResponseWriter, r logger.ResponseEx(service.Name+operationName, ipconfigRequest, reserveResp, reserveResp.Response.ReturnCode, err) } -var errNilStateStore = errors.New("nil endpoint state store") +var errStoreEmpty = errors.New("empty endpoint state store") func (service *HTTPRestService) updateEndpointState(ipconfigRequest cns.IPConfigRequest, podInfo cns.PodInfo, podIPInfo cns.PodIpInfo) error { if service.EndpointStateStore == nil { - return errNilStateStore + return errStoreEmpty } service.Lock() defer service.Unlock() @@ -112,11 +112,23 @@ func (service *HTTPRestService) updateEndpointState(ipconfigRequest cns.IPConfig if err != nil { return fmt.Errorf("failed to parse pod ip address: %w", err) } - ipconfig := &net.IPNet{IP: ip, Mask: ipnet.Mask} + ipconfig := net.IPNet{IP: ip, Mask: ipnet.Mask} if ip.To4() == nil { // is an ipv6 address - endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv6 = ipconfig + for _, ipconf := range endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv6 { + if ipconf.IP.Equal(ipconfig.IP) { + logger.Printf("[updateEndpointState] Found existing ipv6 ipconfig for infra container %s", ipconfigRequest.InfraContainerID) + return nil + } + } + endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv6 = append(endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv6, ipconfig) } else { - endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv4 = ipconfig + for _, ipconf := range endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv4 { + if ipconf.IP.Equal(ipconfig.IP) { + logger.Printf("[updateEndpointState] Found existing ipv4 ipconfig for infra container %s", ipconfigRequest.InfraContainerID) + return nil + } + } + endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv4 = append(endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv4, ipconfig) } service.EndpointState[ipconfigRequest.InfraContainerID] = endpointInfo @@ -127,12 +139,12 @@ func (service *HTTPRestService) updateEndpointState(ipconfigRequest cns.IPConfig if err != nil { return fmt.Errorf("failed to parse pod ip address: %w", err) } - ipconfig := &net.IPNet{IP: ip, Mask: ipnet.Mask} + ipconfig := net.IPNet{IP: ip, Mask: ipnet.Mask} ipInfo := &IPInfo{} if ip.To4() == nil { // is an ipv6 address - ipInfo.IPv6 = ipconfig + ipInfo.IPv6 = append(ipInfo.IPv6, ipconfig) } else { - ipInfo.IPv4 = ipconfig + ipInfo.IPv4 = append(ipInfo.IPv4, ipconfig) } endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname] = ipInfo service.EndpointState[ipconfigRequest.InfraContainerID] = endpointInfo @@ -194,7 +206,7 @@ func (service *HTTPRestService) releaseIPConfigHandler(w http.ResponseWriter, r func (service *HTTPRestService) removeEndpointState(podInfo cns.PodInfo) error { if service.EndpointStateStore == nil { - return errNilStateStore + return errStoreEmpty } service.Lock() defer service.Unlock() diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 04e4f27dfa..7fbef159ee 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -153,12 +153,12 @@ func TestEndpointStateReadAndWrite(t *testing.T) { if err != nil { t.Fatalf("failed to parse pod ip address: %+v", err) } - ipconfig := &net.IPNet{IP: ip, Mask: ipnet.Mask} + ipconfig := net.IPNet{IP: ip, Mask: ipnet.Mask} ipInfo := &IPInfo{} if ip.To4() == nil { // is an ipv6 address - ipInfo.IPv6 = ipconfig + ipInfo.IPv6 = append(ipInfo.IPv6, ipconfig) } else { - ipInfo.IPv4 = ipconfig + ipInfo.IPv4 = append(ipInfo.IPv4, ipconfig) } // add diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index 4ff57f17ed..5106583e66 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -71,8 +71,8 @@ type EndpointInfo struct { } type IPInfo struct { - IPv4 *net.IPNet - IPv6 *net.IPNet + IPv4 []net.IPNet + IPv6 []net.IPNet } type GetHTTPServiceDataResponse struct { From 9f50a499e187014bf8dafa7f5860b0af9736dc5b Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Wed, 17 Aug 2022 15:47:12 -0700 Subject: [PATCH 14/16] update acn tag --- azure-ipam/go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-ipam/go.mod b/azure-ipam/go.mod index 9a1c40b247..3520d7935d 100644 --- a/azure-ipam/go.mod +++ b/azure-ipam/go.mod @@ -74,7 +74,7 @@ require ( ) require ( - github.com/Azure/azure-container-networking v1.4.27 + github.com/Azure/azure-container-networking v1.4.31 github.com/containernetworking/cni v1.1.1 github.com/containernetworking/plugins v1.1.1 go.uber.org/atomic v1.9.0 // indirect From da751eee51e98188195955b44be2a389289104ab Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Wed, 17 Aug 2022 23:57:34 +0000 Subject: [PATCH 15/16] go get on acn --- azure-ipam/go.mod | 14 +++++++------- azure-ipam/go.sum | 11 +++++++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/azure-ipam/go.mod b/azure-ipam/go.mod index 3520d7935d..10f7217106 100644 --- a/azure-ipam/go.mod +++ b/azure-ipam/go.mod @@ -4,7 +4,7 @@ go 1.18 require ( github.com/pkg/errors v0.9.1 - github.com/stretchr/testify v1.7.1 + github.com/stretchr/testify v1.8.0 go.uber.org/zap v1.21.0 ) @@ -60,14 +60,14 @@ require ( google.golang.org/protobuf v1.28.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - gopkg.in/yaml.v3 v3.0.0 // indirect - k8s.io/api v0.24.1 // indirect - k8s.io/apimachinery v0.24.1 // indirect - k8s.io/client-go v0.24.1 // indirect - k8s.io/klog/v2 v2.60.1 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect + k8s.io/api v0.24.2 // indirect + k8s.io/apimachinery v0.24.2 // indirect + k8s.io/client-go v0.24.2 // indirect + k8s.io/klog/v2 v2.70.0 // indirect k8s.io/kube-openapi v0.0.0-20220328201542-3ee0da9b0b42 // indirect k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9 // indirect - sigs.k8s.io/controller-runtime v0.12.1 // indirect + sigs.k8s.io/controller-runtime v0.12.3 // indirect sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.1 // indirect sigs.k8s.io/yaml v1.3.0 // indirect diff --git a/azure-ipam/go.sum b/azure-ipam/go.sum index ee2765ae3f..7cec1355a9 100644 --- a/azure-ipam/go.sum +++ b/azure-ipam/go.sum @@ -42,6 +42,8 @@ code.cloudfoundry.org/clock v1.0.0/go.mod h1:QD9Lzhd/ux6eNQVUDVRJX/RKTigpewimNYB dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= github.com/Azure/azure-container-networking v1.4.27 h1:FtcHxDYMUwtk0fkNix52TSJPjLS40MAsNKZ23zDal5o= github.com/Azure/azure-container-networking v1.4.27/go.mod h1:bdKLmteB7WXZAX2zFBjlifQ+Ewu0joxAOTfzY5uwry8= +github.com/Azure/azure-container-networking v1.4.31 h1:p7N+pHFBb7upSmN3aOMr7VWpmLU5OELHy72iYyjFQb8= +github.com/Azure/azure-container-networking v1.4.31/go.mod h1:m4aNQQVbGBMYhhWrzq5QO33SAGtu4Mam/2vi65I1KoA= github.com/Azure/go-autorest v14.2.0+incompatible/go.mod h1:r+4oMnoxhatjLLJ6zxSWATqVooLgysK6ZNox3g/xq24= github.com/Azure/go-autorest/autorest v0.11.18/go.mod h1:dSiJPy22c3u0OtOKDNttNgqpNFY/GeWa7GH/Pz56QRA= github.com/Azure/go-autorest/autorest/adal v0.9.13/go.mod h1:W/MM4U6nLxnIskrw4UwWzlHfGjwUS50aOsc/I3yuU8M= @@ -249,6 +251,7 @@ github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLe github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= +github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg= github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk= github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= @@ -403,6 +406,7 @@ github.com/spf13/viper v1.4.0/go.mod h1:PTJ7Z/lr49W6bUbkmS1V3by4uWynFiR9p7+dSq/y github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= @@ -411,6 +415,7 @@ github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMTY= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/tedsuo/ifrit v0.0.0-20180802180643-bea94bb476cc/go.mod h1:eyZnKCc955uh98WQvzOm0dgAeLnf2O0Rz0LPoC5ze+0= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc= @@ -827,6 +832,7 @@ gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776/go.mod h1:K4uyk7z7BCEPqu6E+C gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0 h1:hjy8E9ON/egN1tAYqKb61G10WtihqetD4sz2H+8nIeA= gopkg.in/yaml.v3 v3.0.0/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= @@ -837,15 +843,19 @@ honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9 honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= k8s.io/api v0.24.1 h1:BjCMRDcyEYz03joa3K1+rbshwh1Ay6oB53+iUx2H8UY= k8s.io/api v0.24.1/go.mod h1:JhoOvNiLXKTPQ60zh2g0ewpA+bnEYf5q44Flhquh4vQ= +k8s.io/api v0.24.2/go.mod h1:AHqbSkTm6YrQ0ObxjO3Pmp/ubFF/KuM7jU+3khoBsOg= k8s.io/apimachinery v0.24.1 h1:ShD4aDxTQKN5zNf8K1RQ2u98ELLdIW7jEnlO9uAMX/I= k8s.io/apimachinery v0.24.1/go.mod h1:82Bi4sCzVBdpYjyI4jY6aHX+YCUchUIrZrXKedjd2UM= +k8s.io/apimachinery v0.24.2/go.mod h1:82Bi4sCzVBdpYjyI4jY6aHX+YCUchUIrZrXKedjd2UM= k8s.io/client-go v0.24.1 h1:w1hNdI9PFrzu3OlovVeTnf4oHDt+FJLd9Ndluvnb42E= k8s.io/client-go v0.24.1/go.mod h1:f1kIDqcEYmwXS/vTbbhopMUbhKp2JhOeVTfxgaCIlF8= +k8s.io/client-go v0.24.2/go.mod h1:zg4Xaoo+umDsfCWr4fCnmLEtQXyCNXCvJuSsglNcV30= k8s.io/gengo v0.0.0-20210813121822-485abfe95c7c/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE= k8s.io/klog/v2 v2.2.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y= k8s.io/klog/v2 v2.60.1 h1:VW25q3bZx9uE3vvdL6M8ezOX79vA2Aq1nEWLqNQclHc= k8s.io/klog/v2 v2.60.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= +k8s.io/klog/v2 v2.70.0/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= k8s.io/kube-openapi v0.0.0-20220328201542-3ee0da9b0b42 h1:Gii5eqf+GmIEwGNKQYQClCayuJCe2/4fZUvF7VG99sU= k8s.io/kube-openapi v0.0.0-20220328201542-3ee0da9b0b42/go.mod h1:Z/45zLw8lUo4wdiUkI+v/ImEGAvu3WatcZl3lPMR4Rk= k8s.io/utils v0.0.0-20210802155522-efc7438f0176/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= @@ -856,6 +866,7 @@ rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= sigs.k8s.io/controller-runtime v0.12.1 h1:4BJY01xe9zKQti8oRjj/NeHKRXthf1YkYJAgLONFFoI= sigs.k8s.io/controller-runtime v0.12.1/go.mod h1:BKhxlA4l7FPK4AQcsuL4X6vZeWnKDXez/vp1Y8dxTU0= +sigs.k8s.io/controller-runtime v0.12.3/go.mod h1:qKsk4WE6zW2Hfj0G4v10EnNB2jMG1C+NTb8h+DwCoU0= sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 h1:kDi4JBNAsJWfz1aEXhO8Jg87JJaPNLh5tIzYHgStQ9Y= sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2/go.mod h1:B+TnT182UBxE84DiCz4CVE26eOSDAeYCpfDnC2kdKMY= sigs.k8s.io/structured-merge-diff/v4 v4.0.2/go.mod h1:bJZC9H9iH24zzfZ/41RGcq60oK1F7G282QMXDPYydCw= From 1867cfa8760c245f4c34daa1f77b8fbc25c98674 Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Thu, 18 Aug 2022 09:23:32 -0700 Subject: [PATCH 16/16] addressed comments --- cns/logger/cnslogger.go | 11 +++++++++++ cns/logger/log.go | 4 ++++ cns/restserver/ipam.go | 27 +++++++++++++++++---------- cns/service/main.go | 6 ++++-- log/logger.go | 9 +++++++++ 5 files changed, 45 insertions(+), 12 deletions(-) diff --git a/cns/logger/cnslogger.go b/cns/logger/cnslogger.go index 02250d29b5..a19a2bee75 100644 --- a/cns/logger/cnslogger.go +++ b/cns/logger/cnslogger.go @@ -85,6 +85,17 @@ func (c *CNSLogger) Debugf(format string, args ...any) { c.sendTraceInternal(msg) } +func (c *CNSLogger) Warnf(format string, args ...any) { + c.logger.Warnf(format, args...) + + if c.th == nil || c.DisableTraceLogging { + return + } + + msg := fmt.Sprintf(format, args...) + c.sendTraceInternal(msg) +} + func (c *CNSLogger) Errorf(format string, args ...any) { c.logger.Errorf(format, args...) diff --git a/cns/logger/log.go b/cns/logger/log.go index 8dfcadc5c1..aab0befc23 100644 --- a/cns/logger/log.go +++ b/cns/logger/log.go @@ -37,6 +37,10 @@ func Debugf(format string, args ...any) { Log.Debugf(format, args...) } +func Warnf(format string, args ...any) { + Log.Warnf(format, args...) +} + func LogEvent(event aitelemetry.Event) { Log.LogEvent(event) } diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index a8abac332e..15d4e2ad94 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -97,7 +97,10 @@ func (service *HTTPRestService) requestIPConfigHandler(w http.ResponseWriter, r logger.ResponseEx(service.Name+operationName, ipconfigRequest, reserveResp, reserveResp.Response.ReturnCode, err) } -var errStoreEmpty = errors.New("empty endpoint state store") +var ( + errStoreEmpty = errors.New("empty endpoint state store") + errParsePodIPFailed = errors.New("failed to parse pod's ip") +) func (service *HTTPRestService) updateEndpointState(ipconfigRequest cns.IPConfigRequest, podInfo cns.PodInfo, podIPInfo cns.PodIpInfo) error { if service.EndpointStateStore == nil { @@ -107,13 +110,14 @@ func (service *HTTPRestService) updateEndpointState(ipconfigRequest cns.IPConfig defer service.Unlock() logger.Printf("[updateEndpointState] Updating endpoint state for infra container %s", ipconfigRequest.InfraContainerID) if endpointInfo, ok := service.EndpointState[ipconfigRequest.InfraContainerID]; ok { - logger.Printf("[updateEndpointState] Found existing endpoint state for infra container %s", ipconfigRequest.InfraContainerID) - ip, ipnet, err := net.ParseCIDR(podIPInfo.PodIPConfig.IPAddress + "/" + fmt.Sprint(podIPInfo.PodIPConfig.PrefixLength)) - if err != nil { - return fmt.Errorf("failed to parse pod ip address: %w", err) + logger.Warnf("[updateEndpointState] Found existing endpoint state for infra container %s", ipconfigRequest.InfraContainerID) + ip := net.ParseIP(podIPInfo.PodIPConfig.IPAddress) + if ip == nil { + logger.Errorf("failed to parse pod ip address %s", podIPInfo.PodIPConfig.IPAddress) + return errParsePodIPFailed } - ipconfig := net.IPNet{IP: ip, Mask: ipnet.Mask} if ip.To4() == nil { // is an ipv6 address + ipconfig := net.IPNet{IP: ip, Mask: net.CIDRMask(int(podIPInfo.PodIPConfig.PrefixLength), 128)} // nolint for _, ipconf := range endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv6 { if ipconf.IP.Equal(ipconfig.IP) { logger.Printf("[updateEndpointState] Found existing ipv6 ipconfig for infra container %s", ipconfigRequest.InfraContainerID) @@ -122,6 +126,7 @@ func (service *HTTPRestService) updateEndpointState(ipconfigRequest cns.IPConfig } endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv6 = append(endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv6, ipconfig) } else { + ipconfig := net.IPNet{IP: ip, Mask: net.CIDRMask(int(podIPInfo.PodIPConfig.PrefixLength), 32)} // nolint for _, ipconf := range endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname].IPv4 { if ipconf.IP.Equal(ipconfig.IP) { logger.Printf("[updateEndpointState] Found existing ipv4 ipconfig for infra container %s", ipconfigRequest.InfraContainerID) @@ -135,15 +140,17 @@ func (service *HTTPRestService) updateEndpointState(ipconfigRequest cns.IPConfig } else { endpointInfo := &EndpointInfo{PodName: podInfo.Name(), PodNamespace: podInfo.Namespace(), IfnameToIPMap: make(map[string]*IPInfo)} - ip, ipnet, err := net.ParseCIDR(podIPInfo.PodIPConfig.IPAddress + "/" + fmt.Sprint(podIPInfo.PodIPConfig.PrefixLength)) - if err != nil { - return fmt.Errorf("failed to parse pod ip address: %w", err) + ip := net.ParseIP(podIPInfo.PodIPConfig.IPAddress) + if ip == nil { + logger.Errorf("failed to parse pod ip address %s", podIPInfo.PodIPConfig.IPAddress) + return errParsePodIPFailed } - ipconfig := net.IPNet{IP: ip, Mask: ipnet.Mask} ipInfo := &IPInfo{} if ip.To4() == nil { // is an ipv6 address + ipconfig := net.IPNet{IP: ip, Mask: net.CIDRMask(int(podIPInfo.PodIPConfig.PrefixLength), 128)} // nolint ipInfo.IPv6 = append(ipInfo.IPv6, ipconfig) } else { + ipconfig := net.IPNet{IP: ip, Mask: net.CIDRMask(int(podIPInfo.PodIPConfig.PrefixLength), 32)} // nolint ipInfo.IPv4 = append(ipInfo.IPv4, ipconfig) } endpointInfo.IfnameToIPMap[ipconfigRequest.Ifname] = ipInfo diff --git a/cns/service/main.go b/cns/service/main.go index 36439e672c..a13343ec8b 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -548,11 +548,13 @@ func main() { // Initialize endpoint state store if cns is managing endpoint state. if cnsconfig.ManageEndpointState { log.Printf("[Azure CNS] Configured to manage endpoints state") - lockclient, err = processlock.NewFileLock(platform.CNILockPath + endpointStoreName + store.LockExtension) + endpointStoreLock, err := processlock.NewFileLock(platform.CNILockPath + endpointStoreName + store.LockExtension) // nolint if err != nil { log.Printf("Error initializing endpoint state file lock:%v", err) return } + defer endpointStoreLock.Unlock() // nolint + err = platform.CreateDirectory(endpointStoreLocation) if err != nil { logger.Errorf("Failed to create File Store directory %s, due to Error:%v", storeFileLocation, err.Error()) @@ -560,7 +562,7 @@ func main() { } // Create the key value store. storeFileName := endpointStoreLocation + endpointStoreName + ".json" - endpointStateStore, err = store.NewJsonFileStore(storeFileName, lockclient) + endpointStateStore, err = store.NewJsonFileStore(storeFileName, endpointStoreLock) if err != nil { logger.Errorf("Failed to create endpoint state store file: %s, due to error %v\n", storeFileName, err) return diff --git a/log/logger.go b/log/logger.go index 065cd6e064..a1299c7af7 100644 --- a/log/logger.go +++ b/log/logger.go @@ -257,3 +257,12 @@ func (logger *Logger) Debugf(format string, args ...interface{}) { func (logger *Logger) Errorf(format string, args ...interface{}) { logger.Logf(format, args...) } + +// Warnf logs a formatted string at warninglevel +func (logger *Logger) Warnf(format string, args ...interface{}) { + if logger.level < LevelWarning { + return + } + + logger.Logf(format, args...) +}