From ae849be906671484cf6d6e94728d6b77aa2b6f2a Mon Sep 17 00:00:00 2001 From: rejain456 Date: Tue, 22 Oct 2024 17:50:51 +0000 Subject: [PATCH 01/31] Added logic to make 2 hns calls for 2 different endpoint states --- npm/pkg/dataplane/dataplane.go | 24 +++++++++--------- npm/pkg/dataplane/dataplane_windows.go | 34 +++++++++++++++++++++----- 2 files changed, 41 insertions(+), 17 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 4a3ccd68ef..b0d030a521 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -64,12 +64,13 @@ type DataPlane struct { nodeName string // endpointCache stores all endpoints of the network (including off-node) // Key is PodIP - endpointCache *endpointCache - ioShim *common.IOShim - updatePodCache *updatePodCache - endpointQuery *endpointQuery - applyInfo *applyInfo - netPolQueue *netPolQueue + endpointCache *endpointCache + ioShim *common.IOShim + updatePodCache *updatePodCache + endpointQuery *endpointQuery + endpointQueryL1VH *endpointQuery //windows -> filter for state 2 (attached) endpoints in l1vh + applyInfo *applyInfo + netPolQueue *netPolQueue // removePolicyInfo tracks when a policy was removed yet had ApplyIPSet failures. // This field is only relevant for Linux. removePolicyInfo removePolicyInfo @@ -88,11 +89,12 @@ func NewDataPlane(nodeName string, ioShim *common.IOShim, cfg *Config, stopChann policyMgr: policies.NewPolicyManager(ioShim, cfg.PolicyManagerCfg), ipsetMgr: ipsets.NewIPSetManager(cfg.IPSetManagerCfg, ioShim), // networkID is set when initializing Windows dataplane - networkID: "", - endpointCache: newEndpointCache(), - nodeName: nodeName, - ioShim: ioShim, - endpointQuery: new(endpointQuery), + networkID: "", + endpointCache: newEndpointCache(), + nodeName: nodeName, + ioShim: ioShim, + endpointQuery: new(endpointQuery), + endpointQueryL1VH: new(endpointQuery), applyInfo: &applyInfo{ inBootupPhase: true, }, diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index cb65bdd420..0934283d1a 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -50,13 +50,25 @@ func (dp *DataPlane) initializeDataPlane() error { }, Flags: hcn.HostComputeQueryFlagsNone, } + // Initialize Endpoint query used to filter healthy endpoints (vNIC) of Windows pods on L1VH Node + dp.endpointQueryL1VH.query = hcn.HostComputeQuery{ + SchemaVersion: hcn.SchemaVersion{ + Major: hcnSchemaMajorVersion, + Minor: hcnSchemaMinorVersion, + }, + Flags: hcn.HostComputeQueryFlagsNone, + } + // Filter out any endpoints that are not in "AttachedShared" State. All running Windows pods with networking must be in this state. filterMap := map[string]uint16{"State": hcnEndpointStateAttachedSharing} - filter, err := json.Marshal(filterMap) - if err != nil { - return npmerrors.SimpleErrorWrapper("failed to marshal endpoint filter map", err) - } + filterMapL1VH := map[string]uint16{"State": hcnEndpointStateAttached} + + filter, err := marshalFilterMap(filterMap) + + filterL1VH, err := marshalFilterMap(filterMapL1VH) + dp.endpointQuery.query.Filter = string(filter) + dp.endpointQueryL1VH.query.Filter = string(filterL1VH) // reset endpoint cache so that netpol references are removed for all endpoints while refreshing pod endpoints // no need to lock endpointCache at boot up @@ -65,6 +77,14 @@ func (dp *DataPlane) initializeDataPlane() error { return nil } +func marshalFilterMap(filtermap map[string]uint16) ([]byte, error) { + filter, err := json.Marshal(filtermap) + if err != nil { + return nil, npmerrors.SimpleErrorWrapper("failed to marshal endpoint filter map", err) + } + return filter, nil +} + func (dp *DataPlane) getNetworkInfo() error { retryNumber := 0 ticker := time.NewTicker(time.Second * time.Duration(maxNoNetSleepTime)) @@ -330,13 +350,15 @@ func (dp *DataPlane) getEndpointsToApplyPolicies(netPols []*policies.NPMNetworkP func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) { klog.Info("getting local endpoints") timer := metrics.StartNewTimer() - endpoints, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQuery.query) + endpointsAttachedSharing, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQuery.query) + endpointsAttached, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQueryL1VH.query) metrics.RecordListEndpointsLatency(timer) if err != nil { metrics.IncListEndpointsFailures() return nil, npmerrors.SimpleErrorWrapper("failed to get local pod endpoints", err) } - + endpoints := append(endpointsAttachedSharing, endpointsAttached...) + klog.Infof("there are %+v endpoints in endpointsAttachedSharing and %+v endpoints in Attached", len(endpointsAttachedSharing), len(endpointsAttached)) epPointers := make([]*hcn.HostComputeEndpoint, 0, len(endpoints)) for k := range endpoints { epPointers = append(epPointers, &endpoints[k]) From 6e56bba7768df44f582f2734e8e625548958832f Mon Sep 17 00:00:00 2001 From: rejain456 Date: Thu, 24 Oct 2024 17:58:33 +0000 Subject: [PATCH 02/31] added querying to l1vh hns only if npm lite is enabled --- npm/cmd/start.go | 2 ++ npm/pkg/dataplane/dataplane.go | 2 +- npm/pkg/dataplane/dataplane_windows.go | 16 ++++++++++++---- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/npm/cmd/start.go b/npm/cmd/start.go index 391d994f28..5a5dd159d2 100644 --- a/npm/cmd/start.go +++ b/npm/cmd/start.go @@ -137,6 +137,8 @@ func start(config npmconfig.Config, flags npmconfig.Flags) error { stopChannel := wait.NeverStop if config.Toggles.EnableV2NPM { // update the dataplane config + npmV2DataplaneCfg.EnableNPMLite = config.Toggles.EnableV2NPM + npmV2DataplaneCfg.MaxBatchedACLsPerPod = config.MaxBatchedACLsPerPod npmV2DataplaneCfg.NetPolInBackground = config.Toggles.NetPolInBackground diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index b0d030a521..b9cb3190c3 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -45,6 +45,7 @@ type Config struct { NetPolInBackground bool MaxPendingNetPols int NetPolInterval time.Duration + EnableNPMLite bool *ipsets.IPSetManagerCfg *policies.PolicyManagerCfg } @@ -130,7 +131,6 @@ func NewDataPlane(nodeName string, ioShim *common.IOShim, cfg *Config, stopChann } else { metrics.SendLog(util.DaemonDataplaneID, "[DataPlane] dataplane configured to NOT add netpols in background", true) } - return dp, nil } diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 0934283d1a..c619fb6048 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -350,15 +350,23 @@ func (dp *DataPlane) getEndpointsToApplyPolicies(netPols []*policies.NPMNetworkP func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) { klog.Info("getting local endpoints") timer := metrics.StartNewTimer() - endpointsAttachedSharing, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQuery.query) - endpointsAttached, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQueryL1VH.query) + endpoints, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQuery.query) + klog.Infof("There are %+v endpoints in AttachedSharing state", len(endpoints)) metrics.RecordListEndpointsLatency(timer) if err != nil { metrics.IncListEndpointsFailures() return nil, npmerrors.SimpleErrorWrapper("failed to get local pod endpoints", err) } - endpoints := append(endpointsAttachedSharing, endpointsAttached...) - klog.Infof("there are %+v endpoints in endpointsAttachedSharing and %+v endpoints in Attached", len(endpointsAttachedSharing), len(endpointsAttached)) + if dp.EnableNPMLite { + timer = metrics.StartNewTimer() + endpointsAttached, errL1vh := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQueryL1VH.query) + if errL1vh != nil { + metrics.IncListEndpointsFailures() + return nil, npmerrors.SimpleErrorWrapper("failed to get local pod endpoints in L1VH", err) + } + klog.Infof("There are %+v endpoints in Attached state on l1vh", len(endpointsAttached)) + endpoints = append(endpoints, endpointsAttached...) + } epPointers := make([]*hcn.HostComputeEndpoint, 0, len(endpoints)) for k := range endpoints { epPointers = append(epPointers, &endpoints[k]) From 98a5d1ee1a68a0ca13b12a6f7b089c4624a61301 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Thu, 24 Oct 2024 18:07:24 +0000 Subject: [PATCH 03/31] added logging line for debugging --- npm/pkg/dataplane/dataplane_windows.go | 1 + 1 file changed, 1 insertion(+) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index c619fb6048..04ba82aa42 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -357,6 +357,7 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) metrics.IncListEndpointsFailures() return nil, npmerrors.SimpleErrorWrapper("failed to get local pod endpoints", err) } + klog.Infof("NPM liste is enabled: %+v", dp.EnableNPMLite) if dp.EnableNPMLite { timer = metrics.StartNewTimer() endpointsAttached, errL1vh := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQueryL1VH.query) From ddc83b090e7cac92a3f89a8beae888a91be778a3 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Thu, 24 Oct 2024 20:14:31 +0000 Subject: [PATCH 04/31] updated config --- npm/cmd/start.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npm/cmd/start.go b/npm/cmd/start.go index 5a5dd159d2..0969720f94 100644 --- a/npm/cmd/start.go +++ b/npm/cmd/start.go @@ -137,7 +137,7 @@ func start(config npmconfig.Config, flags npmconfig.Flags) error { stopChannel := wait.NeverStop if config.Toggles.EnableV2NPM { // update the dataplane config - npmV2DataplaneCfg.EnableNPMLite = config.Toggles.EnableV2NPM + npmV2DataplaneCfg.EnableNPMLite = config.Toggles.EnableNPMLite npmV2DataplaneCfg.MaxBatchedACLsPerPod = config.MaxBatchedACLsPerPod From 382c1501d9171a157a8a4fb3387db370b4bbeeb1 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Thu, 24 Oct 2024 22:39:34 +0000 Subject: [PATCH 05/31] removed logging lines --- npm/pkg/dataplane/dataplane_windows.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 04ba82aa42..95076a21f1 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -351,13 +351,12 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) klog.Info("getting local endpoints") timer := metrics.StartNewTimer() endpoints, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQuery.query) - klog.Infof("There are %+v endpoints in AttachedSharing state", len(endpoints)) metrics.RecordListEndpointsLatency(timer) if err != nil { metrics.IncListEndpointsFailures() return nil, npmerrors.SimpleErrorWrapper("failed to get local pod endpoints", err) } - klog.Infof("NPM liste is enabled: %+v", dp.EnableNPMLite) + if dp.EnableNPMLite { timer = metrics.StartNewTimer() endpointsAttached, errL1vh := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQueryL1VH.query) @@ -365,7 +364,6 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) metrics.IncListEndpointsFailures() return nil, npmerrors.SimpleErrorWrapper("failed to get local pod endpoints in L1VH", err) } - klog.Infof("There are %+v endpoints in Attached state on l1vh", len(endpointsAttached)) endpoints = append(endpoints, endpointsAttached...) } epPointers := make([]*hcn.HostComputeEndpoint, 0, len(endpoints)) From 1ef66e350c088b94abeb25934b4d7a003e222ee3 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 25 Oct 2024 19:57:26 +0000 Subject: [PATCH 06/31] fixing go lint err --- npm/pkg/dataplane/dataplane.go | 2 +- npm/pkg/dataplane/dataplane_windows.go | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index b9cb3190c3..5bd6588930 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -69,7 +69,7 @@ type DataPlane struct { ioShim *common.IOShim updatePodCache *updatePodCache endpointQuery *endpointQuery - endpointQueryL1VH *endpointQuery //windows -> filter for state 2 (attached) endpoints in l1vh + endpointQueryL1VH *endpointQuery // windows -> filter for state 2 (attached) endpoints in l1vh applyInfo *applyInfo netPolQueue *netPolQueue // removePolicyInfo tracks when a policy was removed yet had ApplyIPSet failures. diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 95076a21f1..3b8a50eefa 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -65,7 +65,13 @@ func (dp *DataPlane) initializeDataPlane() error { filter, err := marshalFilterMap(filterMap) - filterL1VH, err := marshalFilterMap(filterMapL1VH) + filterL1VH, errL1VH := marshalFilterMap(filterMapL1VH) + + if err != nil { + return err + } else if errL1VH != nil { + return errL1VH + } dp.endpointQuery.query.Filter = string(filter) dp.endpointQueryL1VH.query.Filter = string(filterL1VH) @@ -360,6 +366,7 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) if dp.EnableNPMLite { timer = metrics.StartNewTimer() endpointsAttached, errL1vh := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQueryL1VH.query) + metrics.RecordListEndpointsLatency(timer) if errL1vh != nil { metrics.IncListEndpointsFailures() return nil, npmerrors.SimpleErrorWrapper("failed to get local pod endpoints in L1VH", err) From 70fb5c4317936dc855d090fc781be6f29847cb13 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 25 Oct 2024 20:21:59 +0000 Subject: [PATCH 07/31] refactored based on pr comments --- npm/pkg/dataplane/dataplane_windows.go | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 3b8a50eefa..f132b0ff63 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -2,7 +2,6 @@ package dataplane import ( "encoding/json" - "errors" "fmt" "strings" "time" @@ -12,6 +11,7 @@ import ( "github.com/Azure/azure-container-networking/npm/util" npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" "github.com/Microsoft/hcsshim/hcn" + "github.com/pkg/errors" "k8s.io/klog" ) @@ -63,14 +63,14 @@ func (dp *DataPlane) initializeDataPlane() error { filterMap := map[string]uint16{"State": hcnEndpointStateAttachedSharing} filterMapL1VH := map[string]uint16{"State": hcnEndpointStateAttached} - filter, err := marshalFilterMap(filterMap) - - filterL1VH, errL1VH := marshalFilterMap(filterMapL1VH) - + filter, err := json.Marshal(filterMap) if err != nil { - return err - } else if errL1VH != nil { - return errL1VH + return errors.Wrap(err, "failed to marshal endpoint filter map") + } + + filterL1VH, errL1VH := json.Marshal(filterMapL1VH) + if errL1VH != nil { + return errors.Wrap(errL1VH, "failed to marshal endpoint filter map") } dp.endpointQuery.query.Filter = string(filter) @@ -83,14 +83,6 @@ func (dp *DataPlane) initializeDataPlane() error { return nil } -func marshalFilterMap(filtermap map[string]uint16) ([]byte, error) { - filter, err := json.Marshal(filtermap) - if err != nil { - return nil, npmerrors.SimpleErrorWrapper("failed to marshal endpoint filter map", err) - } - return filter, nil -} - func (dp *DataPlane) getNetworkInfo() error { retryNumber := 0 ticker := time.NewTicker(time.Second * time.Duration(maxNoNetSleepTime)) From 605a498e75f4bd1449a3e3841a4ee56108f0062c Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 25 Oct 2024 20:24:42 +0000 Subject: [PATCH 08/31] replaced with errors.Wrap and fixed a logging statement --- npm/pkg/dataplane/dataplane_windows.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index f132b0ff63..e04f3aae00 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -352,7 +352,7 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) metrics.RecordListEndpointsLatency(timer) if err != nil { metrics.IncListEndpointsFailures() - return nil, npmerrors.SimpleErrorWrapper("failed to get local pod endpoints", err) + return nil, errors.Wrap(err, "failed to get local pod endpoints") } if dp.EnableNPMLite { @@ -361,7 +361,7 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) metrics.RecordListEndpointsLatency(timer) if errL1vh != nil { metrics.IncListEndpointsFailures() - return nil, npmerrors.SimpleErrorWrapper("failed to get local pod endpoints in L1VH", err) + return nil, errors.Wrap(errL1vh, "failed to get local pod endpoints in L1VH") } endpoints = append(endpoints, endpointsAttached...) } From 6dde355cad7804288497b550c9dccbde009e0bf9 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 25 Oct 2024 20:29:16 +0000 Subject: [PATCH 09/31] added if condition with logic --- npm/pkg/dataplane/dataplane_windows.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index e04f3aae00..626764513a 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -61,21 +61,21 @@ func (dp *DataPlane) initializeDataPlane() error { // Filter out any endpoints that are not in "AttachedShared" State. All running Windows pods with networking must be in this state. filterMap := map[string]uint16{"State": hcnEndpointStateAttachedSharing} - filterMapL1VH := map[string]uint16{"State": hcnEndpointStateAttached} - filter, err := json.Marshal(filterMap) if err != nil { return errors.Wrap(err, "failed to marshal endpoint filter map") } + dp.endpointQuery.query.Filter = string(filter) - filterL1VH, errL1VH := json.Marshal(filterMapL1VH) - if errL1VH != nil { - return errors.Wrap(errL1VH, "failed to marshal endpoint filter map") + if dp.EnableNPMLite { + filterMapL1VH := map[string]uint16{"State": hcnEndpointStateAttached} + filterL1VH, errL1VH := json.Marshal(filterMapL1VH) + if errL1VH != nil { + return errors.Wrap(errL1VH, "failed to marshal endpoint filter map") + } + dp.endpointQueryL1VH.query.Filter = string(filterL1VH) } - dp.endpointQuery.query.Filter = string(filter) - dp.endpointQueryL1VH.query.Filter = string(filterL1VH) - // reset endpoint cache so that netpol references are removed for all endpoints while refreshing pod endpoints // no need to lock endpointCache at boot up dp.endpointCache.cache = make(map[string]*npmEndpoint) From 4abb1c9c59d519ebea61eca86f4a5c3fdf25e3a1 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 25 Oct 2024 20:45:44 +0000 Subject: [PATCH 10/31] changed errl1vh to err --- npm/pkg/dataplane/dataplane_windows.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 626764513a..55cad4e737 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -69,9 +69,9 @@ func (dp *DataPlane) initializeDataPlane() error { if dp.EnableNPMLite { filterMapL1VH := map[string]uint16{"State": hcnEndpointStateAttached} - filterL1VH, errL1VH := json.Marshal(filterMapL1VH) - if errL1VH != nil { - return errors.Wrap(errL1VH, "failed to marshal endpoint filter map") + filterL1VH, err := json.Marshal(filterMapL1VH) + if err != nil { + return errors.Wrap(err, "failed to marshal endpoint filter map") } dp.endpointQueryL1VH.query.Filter = string(filterL1VH) } @@ -357,11 +357,11 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) if dp.EnableNPMLite { timer = metrics.StartNewTimer() - endpointsAttached, errL1vh := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQueryL1VH.query) + endpointsAttached, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQueryL1VH.query) metrics.RecordListEndpointsLatency(timer) - if errL1vh != nil { + if err != nil { metrics.IncListEndpointsFailures() - return nil, errors.Wrap(errL1vh, "failed to get local pod endpoints in L1VH") + return nil, errors.Wrap(err, "failed to get local pod endpoints in L1VH") } endpoints = append(endpoints, endpointsAttached...) } From f19fd62b836bdf7946e2968e45340f1b079bfcb6 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 25 Oct 2024 20:52:18 +0000 Subject: [PATCH 11/31] added omments --- npm/pkg/dataplane/dataplane_windows.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 55cad4e737..bb8b3ca9bf 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -63,15 +63,16 @@ func (dp *DataPlane) initializeDataPlane() error { filterMap := map[string]uint16{"State": hcnEndpointStateAttachedSharing} filter, err := json.Marshal(filterMap) if err != nil { - return errors.Wrap(err, "failed to marshal endpoint filter map") + return errors.Wrap(err, "failed to marshal endpoint filter map for attachedsharing state") } dp.endpointQuery.query.Filter = string(filter) + // Filter out any endpoints that are not in "Attached" State. All running Windows L1VH pods with networking must be in this state. if dp.EnableNPMLite { filterMapL1VH := map[string]uint16{"State": hcnEndpointStateAttached} filterL1VH, err := json.Marshal(filterMapL1VH) if err != nil { - return errors.Wrap(err, "failed to marshal endpoint filter map") + return errors.Wrap(err, "failed to marshal endpoint filter map for attched state on L1VH Node") } dp.endpointQueryL1VH.query.Filter = string(filterL1VH) } From a79e44f0617d26e62811c89692c2bf44b8e54997 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 25 Oct 2024 21:19:58 +0000 Subject: [PATCH 12/31] added logging lines for debugging --- npm/pkg/dataplane/dataplane_windows.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index bb8b3ca9bf..fb11c7861d 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -350,6 +350,7 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) klog.Info("getting local endpoints") timer := metrics.StartNewTimer() endpoints, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQuery.query) + klog.Infof("There are %+v endpoints with state: attached sharing", len(endpoints)) // Will remove after debugging metrics.RecordListEndpointsLatency(timer) if err != nil { metrics.IncListEndpointsFailures() @@ -359,6 +360,7 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) if dp.EnableNPMLite { timer = metrics.StartNewTimer() endpointsAttached, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQueryL1VH.query) + klog.Infof("There are %+v endpoints with state: attached", len(endpointsAttached)) // Will remove after debugging metrics.RecordListEndpointsLatency(timer) if err != nil { metrics.IncListEndpointsFailures() From 79b54fb205b76032083db28b190078bd16ddef5b Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 25 Oct 2024 21:21:58 +0000 Subject: [PATCH 13/31] added npm lite enabled log debugging --- npm/pkg/dataplane/dataplane_windows.go | 1 + 1 file changed, 1 insertion(+) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index fb11c7861d..a70ec6dcf8 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -348,6 +348,7 @@ func (dp *DataPlane) getEndpointsToApplyPolicies(netPols []*policies.NPMNetworkP func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) { klog.Info("getting local endpoints") + klog.Info("npm lite is enabled: %+v", dp.EnableNPMLite) //Will remove after debugging timer := metrics.StartNewTimer() endpoints, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQuery.query) klog.Infof("There are %+v endpoints with state: attached sharing", len(endpoints)) // Will remove after debugging From 65714fb1529d1d88cb89cbb6855ed9cece59fcbf Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 25 Oct 2024 21:22:14 +0000 Subject: [PATCH 14/31] spacing --- npm/pkg/dataplane/dataplane_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index a70ec6dcf8..7d22bb6b1e 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -348,7 +348,7 @@ func (dp *DataPlane) getEndpointsToApplyPolicies(netPols []*policies.NPMNetworkP func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) { klog.Info("getting local endpoints") - klog.Info("npm lite is enabled: %+v", dp.EnableNPMLite) //Will remove after debugging + klog.Info("npm lite is enabled: %+v", dp.EnableNPMLite) // Will remove after debugging timer := metrics.StartNewTimer() endpoints, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQuery.query) klog.Infof("There are %+v endpoints with state: attached sharing", len(endpoints)) // Will remove after debugging From 7b2e422f5d46a226e5c240570b7f292d4b623a87 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 25 Oct 2024 21:22:29 +0000 Subject: [PATCH 15/31] syntax --- npm/pkg/dataplane/dataplane_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 7d22bb6b1e..bb130708ec 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -348,7 +348,7 @@ func (dp *DataPlane) getEndpointsToApplyPolicies(netPols []*policies.NPMNetworkP func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) { klog.Info("getting local endpoints") - klog.Info("npm lite is enabled: %+v", dp.EnableNPMLite) // Will remove after debugging + klog.Infof("npm lite is enabled: %+v", dp.EnableNPMLite) // Will remove after debugging timer := metrics.StartNewTimer() endpoints, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQuery.query) klog.Infof("There are %+v endpoints with state: attached sharing", len(endpoints)) // Will remove after debugging From 79d19b86278b5009b5fcc070d67f0eebf314ae97 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 25 Oct 2024 21:29:14 +0000 Subject: [PATCH 16/31] added logs for debugging --- npm/pkg/dataplane/dataplane_windows.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index bb130708ec..eb573e03ec 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -351,6 +351,9 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) klog.Infof("npm lite is enabled: %+v", dp.EnableNPMLite) // Will remove after debugging timer := metrics.StartNewTimer() endpoints, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQuery.query) + for _, endpoint1 := range endpoints { + klog.Infof("ID: %s, Name: %+v", endpoint1.Id, endpoint1.Name) + } // Debugging klog.Infof("There are %+v endpoints with state: attached sharing", len(endpoints)) // Will remove after debugging metrics.RecordListEndpointsLatency(timer) if err != nil { @@ -362,6 +365,9 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) timer = metrics.StartNewTimer() endpointsAttached, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQueryL1VH.query) klog.Infof("There are %+v endpoints with state: attached", len(endpointsAttached)) // Will remove after debugging + for _, endpoint := range endpointsAttached { + klog.Infof("ID: %s, Name: %+v", endpoint.Id, endpoint.Name) + } // Debugging metrics.RecordListEndpointsLatency(timer) if err != nil { metrics.IncListEndpointsFailures() From 27b76cda56ecea923fd7c4fcd7763b047f4c85d5 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 25 Oct 2024 23:08:52 +0000 Subject: [PATCH 17/31] optimizing api load --- npm/cmd/start_server.go | 3 +++ npm/pkg/dataplane/dataplane_windows.go | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/npm/cmd/start_server.go b/npm/cmd/start_server.go index 6137902d30..d8ac7e1b68 100644 --- a/npm/cmd/start_server.go +++ b/npm/cmd/start_server.go @@ -81,6 +81,9 @@ func startControlplane(config npmconfig.Config, flags npmconfig.Flags) error { } } + k8sConfig.AcceptContentTypes = "application/vnd.kubernetes.protobuf,application/json" + k8sConfig.ContentType = "application/vnd.kubernetes.protobuf" + // Creates the clientset clientset, err := kubernetes.NewForConfig(k8sConfig) if err != nil { diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index eb573e03ec..ec84d4003f 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -352,7 +352,7 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) timer := metrics.StartNewTimer() endpoints, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQuery.query) for _, endpoint1 := range endpoints { - klog.Infof("ID: %s, Name: %+v", endpoint1.Id, endpoint1.Name) + klog.Infof(" AttachedSharing: ID: %s, Name: %+v", endpoint1.Id, endpoint1.Name) } // Debugging klog.Infof("There are %+v endpoints with state: attached sharing", len(endpoints)) // Will remove after debugging metrics.RecordListEndpointsLatency(timer) @@ -364,7 +364,7 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) if dp.EnableNPMLite { timer = metrics.StartNewTimer() endpointsAttached, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQueryL1VH.query) - klog.Infof("There are %+v endpoints with state: attached", len(endpointsAttached)) // Will remove after debugging + klog.Infof("Attached: There are %+v endpoints with state: attached", len(endpointsAttached)) // Will remove after debugging for _, endpoint := range endpointsAttached { klog.Infof("ID: %s, Name: %+v", endpoint.Id, endpoint.Name) } // Debugging From 7728b319d1425175dd9f730abe0b0d89af46ef74 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Tue, 29 Oct 2024 00:40:40 +0000 Subject: [PATCH 18/31] added function to remove common endpoints --- npm/pkg/dataplane/dataplane_windows.go | 34 +++++++++++++++++- npm/pkg/dataplane/dataplane_windows_test.go | 38 +++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index ec84d4003f..f5ccd3c4ac 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -373,7 +373,8 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) metrics.IncListEndpointsFailures() return nil, errors.Wrap(err, "failed to get local pod endpoints in L1VH") } - endpoints = append(endpoints, endpointsAttached...) + // TODO -> Check if endpoints and endpointsAttached have any same endpoint and if so filter those out + endpoints = removeCommonEndpoints(endpoints, endpointsAttached) } epPointers := make([]*hcn.HostComputeEndpoint, 0, len(endpoints)) for k := range endpoints { @@ -382,6 +383,37 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) return epPointers, nil } +func removeCommonEndpoints(endpoints, endpointsAttached []hcn.HostComputeEndpoint) []hcn.HostComputeEndpoint { + smaller, larger := endpoints, endpointsAttached + if len(endpoints) > len(endpointsAttached) { + smaller, larger = endpointsAttached, endpoints + } + + // Use a map to track the IDs in the smaller array + idMap := make(map[string]struct{}, len(smaller)) + for _, ep := range smaller { + idMap[ep.Id] = struct{}{} + } + + // Collect unique elements from both arrays + var result []hcn.HostComputeEndpoint + for _, ep := range larger { + if _, found := idMap[ep.Id]; !found { + result = append(result, ep) // Unique to larger array + } else { + delete(idMap, ep.Id) // Remove common element from map + } + } + + // Append remaining unique elements from the smaller array + for _, ep := range smaller { + if _, found := idMap[ep.Id]; found { + result = append(result, ep) + } + } + return result +} + // refreshPodEndpoints will refresh all the pod endpoints and create empty netpol references for new endpoints /* Key Assumption: a new pod event (w/ IP) cannot come before HNS knows (and can tell us) about the endpoint. diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index 5cd69a23c2..3ee4077541 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -2,6 +2,7 @@ package dataplane import ( "fmt" + "reflect" "sync" "testing" "time" @@ -10,6 +11,7 @@ import ( "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" + "github.com/Microsoft/hcsshim/hcn" "github.com/pkg/errors" "github.com/stretchr/testify/require" ) @@ -86,6 +88,42 @@ func TestMultiJobApplyInBackground(t *testing.T) { testMultiJobCases(t, multiJobApplyInBackgroundTests(), time.Duration(1*time.Second)) } +func TestRemoveCommonEndpoints(t *testing.T) { + tests := []struct { + name string + endpoints []hcn.HostComputeEndpoint + endpointsAttached []hcn.HostComputeEndpoint + expected []hcn.HostComputeEndpoint + }{ + { + name: "1 value same", + endpoints: []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "123456"}, {Id: "560971"}}, + endpointsAttached: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "123456"}, {Id: "789012"}}, + expected: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "789012"}, {Id: "456901"}, {Id: "560971"}}, + }, + { + name: "no values same", + endpoints: []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "560971"}}, + endpointsAttached: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "789012"}}, + expected: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "789012"}, {Id: "456901"}, {Id: "560971"}}, + }, + } + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + result := removeCommonEndpoints(tt.endpoints, tt.endpointsAttached) + // Use reflect.DeepEqual as a backup if require.Equal doesn't work as expected + if !reflect.DeepEqual(tt.expected, result) { + t.Errorf("Test %s failed: expected %v, got %v", tt.name, tt.expected, result) + } + + // Or, if require.Equal works fine, it will display a descriptive error message + require.Equal(t, tt.expected, result, "expected array equals result") + }) + } +} + func testSerialCases(t *testing.T, tests []*SerialTestCase, finalSleep time.Duration) { for i, tt := range tests { i := i From b6e07fb7c265f464a11a55688621a4d72e6c5769 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Tue, 29 Oct 2024 06:50:01 +0000 Subject: [PATCH 19/31] added logging for debugging --- npm/pkg/dataplane/dataplane_windows.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index f5ccd3c4ac..649c48b242 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -66,6 +66,7 @@ func (dp *DataPlane) initializeDataPlane() error { return errors.Wrap(err, "failed to marshal endpoint filter map for attachedsharing state") } dp.endpointQuery.query.Filter = string(filter) + klog.Infof("Attached Sharing State filter- %+v", string(filter)) // Filter out any endpoints that are not in "Attached" State. All running Windows L1VH pods with networking must be in this state. if dp.EnableNPMLite { @@ -75,6 +76,7 @@ func (dp *DataPlane) initializeDataPlane() error { return errors.Wrap(err, "failed to marshal endpoint filter map for attched state on L1VH Node") } dp.endpointQueryL1VH.query.Filter = string(filterL1VH) + klog.Infof("AttachedState filter- %+v", string(filterL1VH)) } // reset endpoint cache so that netpol references are removed for all endpoints while refreshing pod endpoints @@ -375,6 +377,9 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) } // TODO -> Check if endpoints and endpointsAttached have any same endpoint and if so filter those out endpoints = removeCommonEndpoints(endpoints, endpointsAttached) + for _, endpoint := range endpoints { + klog.Infof("combined enpoints ID: %s, Name: %+v", endpoint.Id, endpoint.Name) + } } epPointers := make([]*hcn.HostComputeEndpoint, 0, len(endpoints)) for k := range endpoints { From b1b941e057810ad3fc2c78ff86d4f5cc328d4225 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Wed, 30 Oct 2024 04:52:25 +0000 Subject: [PATCH 20/31] removed npm lite check --- npm/pkg/dataplane/dataplane.go | 26 +++--- npm/pkg/dataplane/dataplane_windows.go | 90 ++++++++++----------- npm/pkg/dataplane/dataplane_windows_test.go | 16 +++- 3 files changed, 72 insertions(+), 60 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 5bd6588930..5de6f931de 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -65,13 +65,13 @@ type DataPlane struct { nodeName string // endpointCache stores all endpoints of the network (including off-node) // Key is PodIP - endpointCache *endpointCache - ioShim *common.IOShim - updatePodCache *updatePodCache - endpointQuery *endpointQuery - endpointQueryL1VH *endpointQuery // windows -> filter for state 2 (attached) endpoints in l1vh - applyInfo *applyInfo - netPolQueue *netPolQueue + endpointCache *endpointCache + ioShim *common.IOShim + updatePodCache *updatePodCache + endpointQuery *endpointQuery + endpointQueryAttachedState *endpointQuery // windows -> filter for state 2 (attached) endpoints in l1vh + applyInfo *applyInfo + netPolQueue *netPolQueue // removePolicyInfo tracks when a policy was removed yet had ApplyIPSet failures. // This field is only relevant for Linux. removePolicyInfo removePolicyInfo @@ -90,12 +90,12 @@ func NewDataPlane(nodeName string, ioShim *common.IOShim, cfg *Config, stopChann policyMgr: policies.NewPolicyManager(ioShim, cfg.PolicyManagerCfg), ipsetMgr: ipsets.NewIPSetManager(cfg.IPSetManagerCfg, ioShim), // networkID is set when initializing Windows dataplane - networkID: "", - endpointCache: newEndpointCache(), - nodeName: nodeName, - ioShim: ioShim, - endpointQuery: new(endpointQuery), - endpointQueryL1VH: new(endpointQuery), + networkID: "", + endpointCache: newEndpointCache(), + nodeName: nodeName, + ioShim: ioShim, + endpointQuery: new(endpointQuery), + endpointQueryAttachedState: new(endpointQuery), applyInfo: &applyInfo{ inBootupPhase: true, }, diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 649c48b242..54d4d63545 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -51,7 +51,7 @@ func (dp *DataPlane) initializeDataPlane() error { Flags: hcn.HostComputeQueryFlagsNone, } // Initialize Endpoint query used to filter healthy endpoints (vNIC) of Windows pods on L1VH Node - dp.endpointQueryL1VH.query = hcn.HostComputeQuery{ + dp.endpointQueryAttachedState.query = hcn.HostComputeQuery{ SchemaVersion: hcn.SchemaVersion{ Major: hcnSchemaMajorVersion, Minor: hcnSchemaMinorVersion, @@ -69,15 +69,13 @@ func (dp *DataPlane) initializeDataPlane() error { klog.Infof("Attached Sharing State filter- %+v", string(filter)) // Filter out any endpoints that are not in "Attached" State. All running Windows L1VH pods with networking must be in this state. - if dp.EnableNPMLite { - filterMapL1VH := map[string]uint16{"State": hcnEndpointStateAttached} - filterL1VH, err := json.Marshal(filterMapL1VH) - if err != nil { - return errors.Wrap(err, "failed to marshal endpoint filter map for attched state on L1VH Node") - } - dp.endpointQueryL1VH.query.Filter = string(filterL1VH) - klog.Infof("AttachedState filter- %+v", string(filterL1VH)) + filterMapAttached := map[string]uint16{"State": hcnEndpointStateAttached} + filterAttached, err := json.Marshal(filterMapAttached) + if err != nil { + return errors.Wrap(err, "failed to marshal endpoint filter map for attched state on L1VH Node") } + dp.endpointQueryAttachedState.query.Filter = string(filterAttached) + klog.Infof("AttachedState filter- %+v", string(filterAttached)) // reset endpoint cache so that netpol references are removed for all endpoints while refreshing pod endpoints // no need to lock endpointCache at boot up @@ -351,36 +349,38 @@ func (dp *DataPlane) getEndpointsToApplyPolicies(netPols []*policies.NPMNetworkP func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) { klog.Info("getting local endpoints") klog.Infof("npm lite is enabled: %+v", dp.EnableNPMLite) // Will remove after debugging + + // Gets endpoints in state: Attached timer := metrics.StartNewTimer() + endpointsAttached, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQueryAttachedState.query) + klog.Infof("Attached: There are %+v endpoints with state: attached", len(endpointsAttached)) // Will remove after debugging + for _, endpoint := range endpointsAttached { + klog.Infof("ID: %s, Name: %+v", endpoint.Id, endpoint.Name) + } + metrics.RecordListEndpointsLatency(timer) + if err != nil { + metrics.IncListEndpointsFailures() + return nil, errors.Wrap(err, "failed to get local pod endpoints in state:attached") + } + + // Gets endpoints in state: AttachedSharing + timer = metrics.StartNewTimer() endpoints, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQuery.query) for _, endpoint1 := range endpoints { klog.Infof(" AttachedSharing: ID: %s, Name: %+v", endpoint1.Id, endpoint1.Name) - } // Debugging + } klog.Infof("There are %+v endpoints with state: attached sharing", len(endpoints)) // Will remove after debugging metrics.RecordListEndpointsLatency(timer) if err != nil { metrics.IncListEndpointsFailures() - return nil, errors.Wrap(err, "failed to get local pod endpoints") + return nil, errors.Wrap(err, "failed to get local pod endpoints in state: attachedSharing") } - - if dp.EnableNPMLite { - timer = metrics.StartNewTimer() - endpointsAttached, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQueryL1VH.query) - klog.Infof("Attached: There are %+v endpoints with state: attached", len(endpointsAttached)) // Will remove after debugging - for _, endpoint := range endpointsAttached { - klog.Infof("ID: %s, Name: %+v", endpoint.Id, endpoint.Name) - } // Debugging - metrics.RecordListEndpointsLatency(timer) - if err != nil { - metrics.IncListEndpointsFailures() - return nil, errors.Wrap(err, "failed to get local pod endpoints in L1VH") - } - // TODO -> Check if endpoints and endpointsAttached have any same endpoint and if so filter those out - endpoints = removeCommonEndpoints(endpoints, endpointsAttached) - for _, endpoint := range endpoints { - klog.Infof("combined enpoints ID: %s, Name: %+v", endpoint.Id, endpoint.Name) - } + // Filtering out any of the same endpoints between endpoints with state attached and attachedSharing + endpoints = removeCommonEndpoints(&endpoints, &endpointsAttached) + for _, endpoint := range endpoints { + klog.Infof("combined enpoints ID: %s, Name: %+v", endpoint.Id, endpoint.Name) } + epPointers := make([]*hcn.HostComputeEndpoint, 0, len(endpoints)) for k := range endpoints { epPointers = append(epPointers, &endpoints[k]) @@ -388,30 +388,30 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) return epPointers, nil } -func removeCommonEndpoints(endpoints, endpointsAttached []hcn.HostComputeEndpoint) []hcn.HostComputeEndpoint { - smaller, larger := endpoints, endpointsAttached - if len(endpoints) > len(endpointsAttached) { - smaller, larger = endpointsAttached, endpoints +func removeCommonEndpoints(endpoints, endpointsAttached *[]hcn.HostComputeEndpoint) []hcn.HostComputeEndpoint { + smallerEndpointsList, largerEndpointsList := endpoints, endpointsAttached + if len(*endpoints) > len(*endpointsAttached) { + smallerEndpointsList, largerEndpointsList = endpointsAttached, endpoints } - // Use a map to track the IDs in the smaller array - idMap := make(map[string]struct{}, len(smaller)) - for _, ep := range smaller { + // add endpoints from smaller array into a map + idMap := make(map[string]struct{}, len(*smallerEndpointsList)) + for i := 0; i < len(*smallerEndpointsList); i++ { + ep := &(*smallerEndpointsList)[i] idMap[ep.Id] = struct{}{} } - // Collect unique elements from both arrays + // checking for common endpoints among two endpoint arrays var result []hcn.HostComputeEndpoint - for _, ep := range larger { - if _, found := idMap[ep.Id]; !found { - result = append(result, ep) // Unique to larger array - } else { - delete(idMap, ep.Id) // Remove common element from map - } + for i := 0; i < len(*largerEndpointsList); i++ { + ep := (*largerEndpointsList)[i] + result = append(result, ep) + delete(idMap, ep.Id) } - // Append remaining unique elements from the smaller array - for _, ep := range smaller { + // Appending remaining unique elements from the smaller endpoint array + for i := 0; i < len(*smallerEndpointsList); i++ { + ep := (*smallerEndpointsList)[i] if _, found := idMap[ep.Id]; found { result = append(result, ep) } diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index 3ee4077541..24baab7a38 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -99,7 +99,7 @@ func TestRemoveCommonEndpoints(t *testing.T) { name: "1 value same", endpoints: []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "123456"}, {Id: "560971"}}, endpointsAttached: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "123456"}, {Id: "789012"}}, - expected: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "789012"}, {Id: "456901"}, {Id: "560971"}}, + expected: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "123456"}, {Id: "789012"}, {Id: "456901"}, {Id: "560971"}}, }, { name: "no values same", @@ -107,12 +107,24 @@ func TestRemoveCommonEndpoints(t *testing.T) { endpointsAttached: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "789012"}}, expected: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "789012"}, {Id: "456901"}, {Id: "560971"}}, }, + { + name: "1 value same", + endpoints: []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "123456"}, {Id: "560971"}}, + endpointsAttached: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "123456"}, {Id: "789012"}}, + expected: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "123456"}, {Id: "789012"}, {Id: "456901"}, {Id: "560971"}}, + }, + { + name: "two values same", + endpoints: []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "560971"}, {Id: "123456"}, {Id: "789012"}}, + endpointsAttached: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "789012"}, {Id: "123456"}}, + expected: []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "560971"}, {Id: "123456"}, {Id: "789012"}, {Id: "567890"}}, + }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - result := removeCommonEndpoints(tt.endpoints, tt.endpointsAttached) + result := removeCommonEndpoints(&tt.endpoints, &tt.endpointsAttached) // Use reflect.DeepEqual as a backup if require.Equal doesn't work as expected if !reflect.DeepEqual(tt.expected, result) { t.Errorf("Test %s failed: expected %v, got %v", tt.name, tt.expected, result) From ae341d50a87cdf6ca023493e72be0664449c3944 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Thu, 31 Oct 2024 06:22:44 +0000 Subject: [PATCH 21/31] removed all the debugging comments --- npm/pkg/dataplane/dataplane_windows.go | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 54d4d63545..36c4811ede 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -66,7 +66,6 @@ func (dp *DataPlane) initializeDataPlane() error { return errors.Wrap(err, "failed to marshal endpoint filter map for attachedsharing state") } dp.endpointQuery.query.Filter = string(filter) - klog.Infof("Attached Sharing State filter- %+v", string(filter)) // Filter out any endpoints that are not in "Attached" State. All running Windows L1VH pods with networking must be in this state. filterMapAttached := map[string]uint16{"State": hcnEndpointStateAttached} @@ -75,7 +74,6 @@ func (dp *DataPlane) initializeDataPlane() error { return errors.Wrap(err, "failed to marshal endpoint filter map for attched state on L1VH Node") } dp.endpointQueryAttachedState.query.Filter = string(filterAttached) - klog.Infof("AttachedState filter- %+v", string(filterAttached)) // reset endpoint cache so that netpol references are removed for all endpoints while refreshing pod endpoints // no need to lock endpointCache at boot up @@ -348,15 +346,10 @@ func (dp *DataPlane) getEndpointsToApplyPolicies(netPols []*policies.NPMNetworkP func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) { klog.Info("getting local endpoints") - klog.Infof("npm lite is enabled: %+v", dp.EnableNPMLite) // Will remove after debugging // Gets endpoints in state: Attached timer := metrics.StartNewTimer() endpointsAttached, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQueryAttachedState.query) - klog.Infof("Attached: There are %+v endpoints with state: attached", len(endpointsAttached)) // Will remove after debugging - for _, endpoint := range endpointsAttached { - klog.Infof("ID: %s, Name: %+v", endpoint.Id, endpoint.Name) - } metrics.RecordListEndpointsLatency(timer) if err != nil { metrics.IncListEndpointsFailures() @@ -366,20 +359,14 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) // Gets endpoints in state: AttachedSharing timer = metrics.StartNewTimer() endpoints, err := dp.ioShim.Hns.ListEndpointsQuery(dp.endpointQuery.query) - for _, endpoint1 := range endpoints { - klog.Infof(" AttachedSharing: ID: %s, Name: %+v", endpoint1.Id, endpoint1.Name) - } - klog.Infof("There are %+v endpoints with state: attached sharing", len(endpoints)) // Will remove after debugging metrics.RecordListEndpointsLatency(timer) if err != nil { metrics.IncListEndpointsFailures() return nil, errors.Wrap(err, "failed to get local pod endpoints in state: attachedSharing") } + // Filtering out any of the same endpoints between endpoints with state attached and attachedSharing endpoints = removeCommonEndpoints(&endpoints, &endpointsAttached) - for _, endpoint := range endpoints { - klog.Infof("combined enpoints ID: %s, Name: %+v", endpoint.Id, endpoint.Name) - } epPointers := make([]*hcn.HostComputeEndpoint, 0, len(endpoints)) for k := range endpoints { @@ -389,19 +376,20 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) } func removeCommonEndpoints(endpoints, endpointsAttached *[]hcn.HostComputeEndpoint) []hcn.HostComputeEndpoint { + // Choose smaller and larger lists based on length for efficiency smallerEndpointsList, largerEndpointsList := endpoints, endpointsAttached if len(*endpoints) > len(*endpointsAttached) { smallerEndpointsList, largerEndpointsList = endpointsAttached, endpoints } - // add endpoints from smaller array into a map + // Store IDs of smaller list in a map for quick lookup idMap := make(map[string]struct{}, len(*smallerEndpointsList)) for i := 0; i < len(*smallerEndpointsList); i++ { ep := &(*smallerEndpointsList)[i] idMap[ep.Id] = struct{}{} } - // checking for common endpoints among two endpoint arrays + // Append endpoints from larger list and remove common IDs from map var result []hcn.HostComputeEndpoint for i := 0; i < len(*largerEndpointsList); i++ { ep := (*largerEndpointsList)[i] @@ -409,7 +397,7 @@ func removeCommonEndpoints(endpoints, endpointsAttached *[]hcn.HostComputeEndpoi delete(idMap, ep.Id) } - // Appending remaining unique elements from the smaller endpoint array + // Append remaining unique endpoints from smaller list to result for i := 0; i < len(*smallerEndpointsList); i++ { ep := (*smallerEndpointsList)[i] if _, found := idMap[ep.Id]; found { From c5b18be3165c540eec5cc4ffd8c32cf43b34674d Mon Sep 17 00:00:00 2001 From: rejain456 Date: Thu, 31 Oct 2024 06:38:53 +0000 Subject: [PATCH 22/31] added extra unit test cases --- npm/pkg/dataplane/dataplane_windows.go | 2 +- npm/pkg/dataplane/dataplane_windows_test.go | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 36c4811ede..e5bb3548fc 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -390,7 +390,7 @@ func removeCommonEndpoints(endpoints, endpointsAttached *[]hcn.HostComputeEndpoi } // Append endpoints from larger list and remove common IDs from map - var result []hcn.HostComputeEndpoint + result := []hcn.HostComputeEndpoint{} for i := 0; i < len(*largerEndpointsList); i++ { ep := (*largerEndpointsList)[i] result = append(result, ep) diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index 24baab7a38..36534b1d70 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -119,6 +119,24 @@ func TestRemoveCommonEndpoints(t *testing.T) { endpointsAttached: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "789012"}, {Id: "123456"}}, expected: []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "560971"}, {Id: "123456"}, {Id: "789012"}, {Id: "567890"}}, }, + { + name: "no values", + endpoints: []hcn.HostComputeEndpoint{}, + endpointsAttached: []hcn.HostComputeEndpoint{}, + expected: []hcn.HostComputeEndpoint{}, + }, + { + name: "1 value - same", + endpoints: []hcn.HostComputeEndpoint{{Id: "456901"}}, + endpointsAttached: []hcn.HostComputeEndpoint{{Id: "456901"}}, + expected: []hcn.HostComputeEndpoint{{Id: "456901"}}, + }, + { + name: "1 value - different", + endpoints: []hcn.HostComputeEndpoint{{Id: "456901"}}, + endpointsAttached: []hcn.HostComputeEndpoint{{Id: "456990"}}, + expected: []hcn.HostComputeEndpoint{{Id: "456990"}, {Id: "456901"}}, + }, } for _, tt := range tests { tt := tt From 3c237ba9288f212e55f742c5d9ba6747786684ac Mon Sep 17 00:00:00 2001 From: rejain456 Date: Thu, 31 Oct 2024 06:39:55 +0000 Subject: [PATCH 23/31] added additional unit tests --- npm/pkg/dataplane/dataplane_windows_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index 36534b1d70..20272c5feb 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -134,8 +134,8 @@ func TestRemoveCommonEndpoints(t *testing.T) { { name: "1 value - different", endpoints: []hcn.HostComputeEndpoint{{Id: "456901"}}, - endpointsAttached: []hcn.HostComputeEndpoint{{Id: "456990"}}, - expected: []hcn.HostComputeEndpoint{{Id: "456990"}, {Id: "456901"}}, + endpointsAttached: []hcn.HostComputeEndpoint{}, + expected: []hcn.HostComputeEndpoint{{Id: "456901"}}, }, } for _, tt := range tests { From 3089651438c680c10a496e70cbc3b254557be565 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Thu, 31 Oct 2024 06:41:01 +0000 Subject: [PATCH 24/31] removed protobuf code --- npm/cmd/start_server.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/npm/cmd/start_server.go b/npm/cmd/start_server.go index d8ac7e1b68..6137902d30 100644 --- a/npm/cmd/start_server.go +++ b/npm/cmd/start_server.go @@ -81,9 +81,6 @@ func startControlplane(config npmconfig.Config, flags npmconfig.Flags) error { } } - k8sConfig.AcceptContentTypes = "application/vnd.kubernetes.protobuf,application/json" - k8sConfig.ContentType = "application/vnd.kubernetes.protobuf" - // Creates the clientset clientset, err := kubernetes.NewForConfig(k8sConfig) if err != nil { From 15a1182366de26659b0e394e8b4437b879ffa11e Mon Sep 17 00:00:00 2001 From: rejain456 Date: Mon, 4 Nov 2024 18:04:12 +0000 Subject: [PATCH 25/31] fixed comment --- npm/pkg/dataplane/dataplane_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index e5bb3548fc..3944105d6a 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -71,7 +71,7 @@ func (dp *DataPlane) initializeDataPlane() error { filterMapAttached := map[string]uint16{"State": hcnEndpointStateAttached} filterAttached, err := json.Marshal(filterMapAttached) if err != nil { - return errors.Wrap(err, "failed to marshal endpoint filter map for attched state on L1VH Node") + return errors.Wrap(err, "failed to marshal endpoint filter map for attched state") } dp.endpointQueryAttachedState.query.Filter = string(filterAttached) From fd985c73e741b8a1576ed010b84cb61aeb6ded6e Mon Sep 17 00:00:00 2001 From: rejain456 Date: Mon, 4 Nov 2024 18:06:03 +0000 Subject: [PATCH 26/31] fixed a spelling error --- npm/npm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npm/npm.go b/npm/npm.go index 72a429b7d8..914809b437 100644 --- a/npm/npm.go +++ b/npm/npm.go @@ -192,7 +192,7 @@ func (npMgr *NetworkPolicyManager) Start(config npmconfig.Config, stopCh <-chan // Starts all informers manufactured by npMgr's informerFactory. npMgr.InformerFactory.Start(stopCh) - // npn lite + // npm lite if npMgr.NpmLiteToggle { npMgr.PodInformerFactory.Start(stopCh) } From 60dda1cd992db8c66863684a40d3e74660a9b7d0 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Mon, 4 Nov 2024 22:20:50 +0000 Subject: [PATCH 27/31] resolved pr comments --- npm/pkg/dataplane/dataplane_windows.go | 38 +++++++-------------- npm/pkg/dataplane/dataplane_windows_test.go | 16 ++++----- 2 files changed, 18 insertions(+), 36 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 3944105d6a..9f2144a5c5 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -366,7 +366,7 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) } // Filtering out any of the same endpoints between endpoints with state attached and attachedSharing - endpoints = removeCommonEndpoints(&endpoints, &endpointsAttached) + endpoints = GetUniqueEndpoints(endpoints, endpointsAttached) epPointers := make([]*hcn.HostComputeEndpoint, 0, len(endpoints)) for k := range endpoints { @@ -375,36 +375,22 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) return epPointers, nil } -func removeCommonEndpoints(endpoints, endpointsAttached *[]hcn.HostComputeEndpoint) []hcn.HostComputeEndpoint { - // Choose smaller and larger lists based on length for efficiency - smallerEndpointsList, largerEndpointsList := endpoints, endpointsAttached - if len(*endpoints) > len(*endpointsAttached) { - smallerEndpointsList, largerEndpointsList = endpointsAttached, endpoints - } - - // Store IDs of smaller list in a map for quick lookup - idMap := make(map[string]struct{}, len(*smallerEndpointsList)) - for i := 0; i < len(*smallerEndpointsList); i++ { - ep := &(*smallerEndpointsList)[i] +func GetUniqueEndpoints(endpoints, endpointsAttached []hcn.HostComputeEndpoint) []hcn.HostComputeEndpoint { + // Store IDs of endpoints list in a map for quick lookup + idMap := make(map[string]struct{}, len(endpoints)) + for i := 0; i < len(endpoints); i++ { + ep := &(endpoints)[i] idMap[ep.Id] = struct{}{} } - // Append endpoints from larger list and remove common IDs from map - result := []hcn.HostComputeEndpoint{} - for i := 0; i < len(*largerEndpointsList); i++ { - ep := (*largerEndpointsList)[i] - result = append(result, ep) - delete(idMap, ep.Id) - } - - // Append remaining unique endpoints from smaller list to result - for i := 0; i < len(*smallerEndpointsList); i++ { - ep := (*smallerEndpointsList)[i] - if _, found := idMap[ep.Id]; found { - result = append(result, ep) + // Add endpointsAttached list endpoints in endpoints list if the endpoint is not in the map + for i := 0; i < len(endpointsAttached); i++ { + ep := endpointsAttached[i] + if _, ok := idMap[ep.Id]; !ok { + endpoints = append(endpoints, ep) } } - return result + return endpoints } // refreshPodEndpoints will refresh all the pod endpoints and create empty netpol references for new endpoints diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index 20272c5feb..bfd2dd11f3 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -2,7 +2,6 @@ package dataplane import ( "fmt" - "reflect" "sync" "testing" "time" @@ -12,6 +11,7 @@ import ( "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" "github.com/Microsoft/hcsshim/hcn" + "github.com/google/go-cmp/cmp" "github.com/pkg/errors" "github.com/stretchr/testify/require" ) @@ -99,19 +99,19 @@ func TestRemoveCommonEndpoints(t *testing.T) { name: "1 value same", endpoints: []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "123456"}, {Id: "560971"}}, endpointsAttached: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "123456"}, {Id: "789012"}}, - expected: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "123456"}, {Id: "789012"}, {Id: "456901"}, {Id: "560971"}}, + expected: []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "123456"}, {Id: "560971"}, {Id: "567890"}, {Id: "789012"}}, }, { name: "no values same", endpoints: []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "560971"}}, endpointsAttached: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "789012"}}, - expected: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "789012"}, {Id: "456901"}, {Id: "560971"}}, + expected: []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "560971"}, {Id: "567890"}, {Id: "789012"}}, }, { name: "1 value same", endpoints: []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "123456"}, {Id: "560971"}}, endpointsAttached: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "123456"}, {Id: "789012"}}, - expected: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "123456"}, {Id: "789012"}, {Id: "456901"}, {Id: "560971"}}, + expected: []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "123456"}, {Id: "560971"}, {Id: "567890"}, {Id: "789012"}}, }, { name: "two values same", @@ -142,14 +142,10 @@ func TestRemoveCommonEndpoints(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { - result := removeCommonEndpoints(&tt.endpoints, &tt.endpointsAttached) - // Use reflect.DeepEqual as a backup if require.Equal doesn't work as expected - if !reflect.DeepEqual(tt.expected, result) { + result := GetUniqueEndpoints(tt.endpoints, tt.endpointsAttached) + if !cmp.Equal(tt.expected, result) { t.Errorf("Test %s failed: expected %v, got %v", tt.name, tt.expected, result) } - - // Or, if require.Equal works fine, it will display a descriptive error message - require.Equal(t, tt.expected, result, "expected array equals result") }) } } From 36f0d7408ce47e389367db1f25eaf0e5edc49572 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Mon, 4 Nov 2024 22:27:08 +0000 Subject: [PATCH 28/31] updated a comment --- npm/pkg/dataplane/dataplane_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 9f2144a5c5..67ef497b18 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -67,7 +67,7 @@ func (dp *DataPlane) initializeDataPlane() error { } dp.endpointQuery.query.Filter = string(filter) - // Filter out any endpoints that are not in "Attached" State. All running Windows L1VH pods with networking must be in this state. + // Filter out any endpoints that are not in "Attached" State. All running Windows pods on L1VH with networking must be in this state. filterMapAttached := map[string]uint16{"State": hcnEndpointStateAttached} filterAttached, err := json.Marshal(filterMapAttached) if err != nil { From ab7038d546d21513a2c3103b8708c84c34173af8 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Mon, 4 Nov 2024 22:30:16 +0000 Subject: [PATCH 29/31] revised comment --- npm/pkg/dataplane/dataplane_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 67ef497b18..e448b64858 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -365,7 +365,7 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) return nil, errors.Wrap(err, "failed to get local pod endpoints in state: attachedSharing") } - // Filtering out any of the same endpoints between endpoints with state attached and attachedSharing + // Get endpoints unique to endpoints and endpointsAttached endpoints = GetUniqueEndpoints(endpoints, endpointsAttached) epPointers := make([]*hcn.HostComputeEndpoint, 0, len(endpoints)) From 2c66d63a33510569bbbfe93ae706a1753aaccd07 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Mon, 4 Nov 2024 22:59:34 +0000 Subject: [PATCH 30/31] resolved further pr comments --- npm/pkg/dataplane/dataplane_windows.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index e448b64858..9a03509691 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -378,14 +378,12 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) func GetUniqueEndpoints(endpoints, endpointsAttached []hcn.HostComputeEndpoint) []hcn.HostComputeEndpoint { // Store IDs of endpoints list in a map for quick lookup idMap := make(map[string]struct{}, len(endpoints)) - for i := 0; i < len(endpoints); i++ { - ep := &(endpoints)[i] + for _, ep := range endpoints { idMap[ep.Id] = struct{}{} } // Add endpointsAttached list endpoints in endpoints list if the endpoint is not in the map - for i := 0; i < len(endpointsAttached); i++ { - ep := endpointsAttached[i] + for _, ep := range endpointsAttached { if _, ok := idMap[ep.Id]; !ok { endpoints = append(endpoints, ep) } From 10d116f90bd71ecbc65625c0b67594b1773094d7 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Mon, 4 Nov 2024 23:13:43 +0000 Subject: [PATCH 31/31] changed back to for loop from range --- npm/pkg/dataplane/dataplane_windows.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 9a03509691..43af8bef6b 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -378,12 +378,14 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error) func GetUniqueEndpoints(endpoints, endpointsAttached []hcn.HostComputeEndpoint) []hcn.HostComputeEndpoint { // Store IDs of endpoints list in a map for quick lookup idMap := make(map[string]struct{}, len(endpoints)) - for _, ep := range endpoints { + for i := 0; i < len(endpoints); i++ { + ep := endpoints[i] idMap[ep.Id] = struct{}{} } // Add endpointsAttached list endpoints in endpoints list if the endpoint is not in the map - for _, ep := range endpointsAttached { + for i := 0; i < len(endpointsAttached); i++ { + ep := endpointsAttached[i] if _, ok := idMap[ep.Id]; !ok { endpoints = append(endpoints, ep) }