From 1c2c9c66815d600ac1243b83cd69b31933e1304d Mon Sep 17 00:00:00 2001 From: Apoorva Date: Fri, 13 Aug 2021 16:49:47 -0700 Subject: [PATCH] Introduce new lock to safeguard concurrent hostagent code The cache used to store snat policy labels is being accessed by multiple routines, so a new lock is introduced to safely read and write from the map Also, delete two conf files added by mistake in an earlier commit (3c3b56a057bca1f5ddfdf75dde4426a03240e662) --- pkg/hostagent/01-base.conf | 27 ---------------------- pkg/hostagent/10-renderer.conf | 22 ------------------ pkg/hostagent/agent.go | 42 ++++++++++++++++++++++++++++++++++ pkg/hostagent/snats.go | 26 +++++++++------------ 4 files changed, 53 insertions(+), 64 deletions(-) delete mode 100644 pkg/hostagent/01-base.conf delete mode 100644 pkg/hostagent/10-renderer.conf diff --git a/pkg/hostagent/01-base.conf b/pkg/hostagent/01-base.conf deleted file mode 100644 index 15fb15fb73..0000000000 --- a/pkg/hostagent/01-base.conf +++ /dev/null @@ -1,27 +0,0 @@ -{ - "opflex": { - "name": "node1", - "domain": "comp/prov-/ctrlr-[]-/sw-InsiemeLSOid", - "peers": [ - {"hostname": "", "port": "8009"} - ] - } , - "endpoint-sources": { - "filesystem": [""] - }, - "service-sources": { - "filesystem": [""] - }, - "snat-sources": { - "filesystem": [""] - }, - "drop-log-config-sources": { - "filesystem": [""] - }, - "packet-event-notif": { - "socket-name": [""] - }, - "host-agent-fault-sources": { - "filesystem": [""] - } -} diff --git a/pkg/hostagent/10-renderer.conf b/pkg/hostagent/10-renderer.conf deleted file mode 100644 index 206e371196..0000000000 --- a/pkg/hostagent/10-renderer.conf +++ /dev/null @@ -1,22 +0,0 @@ -{ - "renderers": { - "stitched-mode": { - "int-bridge-name": "", - "access-bridge-name": "", - "encap": { - "vlan" : { - "encap-iface": "eth1" - } - }, - "flowid-cache-dir": "", - "mcast-group-file": "", - "drop-log": { - "geneve" : { - "int-br-iface": "", - "access-br-iface": "", - "remote-ip": "" - } - } - } - } -} diff --git a/pkg/hostagent/agent.go b/pkg/hostagent/agent.go index e20b997afc..66370c12e4 100644 --- a/pkg/hostagent/agent.go +++ b/pkg/hostagent/agent.go @@ -47,6 +47,7 @@ type HostAgent struct { indexMutex sync.Mutex ipamMutex sync.Mutex + snatMutex sync.RWMutex opflexEps map[string][]*opflexEndpoint opflexServices map[string]*opflexService @@ -231,6 +232,47 @@ func addPodRoute(ipn types.IPNet, dev string, src string) error { return netlink.RouteAdd(&route) } +func (agent *HostAgent) ReadSnatPolicyLabel(key string) (map[string]ResourceType, bool) { + agent.snatMutex.RLock() + defer agent.snatMutex.RUnlock() + value, ok := agent.snatPolicyLabels[key] + return value, ok +} + +func (agent *HostAgent) WriteSnatPolicyLabel(key string, policy string, res ResourceType) { + agent.snatMutex.Lock() + defer agent.snatMutex.Unlock() + agent.snatPolicyLabels[key][policy] = res +} + +func (agent *HostAgent) WriteNewSnatPolicyLabel(key string) { + agent.snatMutex.Lock() + defer agent.snatMutex.Unlock() + agent.snatPolicyLabels[key] = make(map[string]ResourceType) +} + +func (agent *HostAgent) DeleteSnatPolicyLabelEntry(key string, policy string) { + agent.snatMutex.Lock() + defer agent.snatMutex.Unlock() + delete(agent.snatPolicyLabels[key], policy) +} + +func (agent *HostAgent) DeleteSnatPolicyLabel(key string) { + agent.snatMutex.Lock() + defer agent.snatMutex.Unlock() + delete(agent.snatPolicyLabels, key) +} + +func (agent *HostAgent) DeleteMatchingSnatPolicyLabel(policy string) { + agent.snatMutex.Lock() + defer agent.snatMutex.Unlock() + for key, v := range agent.snatPolicyLabels { + if _, ok := v[policy]; ok { + delete(agent.snatPolicyLabels[key], policy) + } + } +} + func (agent *HostAgent) Init() { agent.log.Debug("Initializing endpoint CNI metadata") err := md.LoadMetadata(agent.config.CniMetadataDir, diff --git a/pkg/hostagent/snats.go b/pkg/hostagent/snats.go index e13c06cb60..970f032a5c 100644 --- a/pkg/hostagent/snats.go +++ b/pkg/hostagent/snats.go @@ -332,9 +332,9 @@ func (agent *HostAgent) handleSnatUpdate(policy *snatpolicy.SnatPolicy) { poduids = append(poduids, uids...) key, err := cache.MetaNamespaceKeyFunc(service) if err == nil { - _, ok := agent.snatPolicyLabels[key] + _, ok := agent.ReadSnatPolicyLabel(key) if ok && len(policy.Spec.Selector.Labels) > 0 { - agent.snatPolicyLabels[key][policy.ObjectMeta.Name] = SERVICE + agent.WriteSnatPolicyLabel(key, policy.ObjectMeta.Name, SERVICE) } } } @@ -374,8 +374,8 @@ func (agent *HostAgent) updateSnatPolicyLabels(obj interface{}, policyname strin uids, res := agent.getPodsMatchingObjet(obj, policyname) if len(uids) > 0 { key, _ := cache.MetaNamespaceKeyFunc(obj) - if _, ok := agent.snatPolicyLabels[key]; ok { - agent.snatPolicyLabels[key][policyname] = res + if _, ok := agent.ReadSnatPolicyLabel(key); ok { + agent.WriteSnatPolicyLabel(key, policyname, res) } } return uids @@ -488,11 +488,7 @@ func (agent *HostAgent) deletePolicy(policy *snatpolicy.SnatPolicy) { delete(agent.snatPods, policy.GetName()) agent.log.Info("SnatPolicy deleted update Nodeinfo: ", policy.GetName()) agent.scheduleSyncNodeInfo() - for key, v := range agent.snatPolicyLabels { - if _, ok := v[policy.GetName()]; ok { - delete(agent.snatPolicyLabels[key], policy.GetName()) - } - } + agent.DeleteMatchingSnatPolicyLabel(policy.GetName()) return } @@ -1051,9 +1047,9 @@ func (agent *HostAgent) handleObjectUpdateForSnat(obj interface{}) { if err != nil { return } - plcynames, ok := agent.snatPolicyLabels[objKey] + plcynames, ok := agent.ReadSnatPolicyLabel(objKey) if !ok { - agent.snatPolicyLabels[objKey] = make(map[string]ResourceType) + agent.WriteNewSnatPolicyLabel(objKey) } sync := false if len(plcynames) == 0 { @@ -1065,7 +1061,7 @@ func (agent *HostAgent) handleObjectUpdateForSnat(obj interface{}) { agent.applyPolicy(poduids, res, name) } else { agent.applyPolicy(poduids, res, name) - agent.snatPolicyLabels[objKey][name] = res + agent.WriteSnatPolicyLabel(objKey, name, res) } if len(poduids) > 0 { sync = true @@ -1085,7 +1081,7 @@ func (agent *HostAgent) handleObjectUpdateForSnat(obj interface{}) { agent.deleteSnatLocalInfo(uid, res, name) } delpodlist = append(delpodlist, poduids...) - delete(agent.snatPolicyLabels[objKey], name) + agent.DeleteSnatPolicyLabelEntry(objKey, name) } seen[name] = true } @@ -1100,7 +1096,7 @@ func (agent *HostAgent) handleObjectUpdateForSnat(obj interface{}) { for _, res := range resources { poduids, _ := agent.getPodsMatchingObjet(obj, name) agent.applyPolicy(poduids, res, name) - agent.snatPolicyLabels[objKey][name] = res + agent.WriteSnatPolicyLabel(objKey, name, res) sync = true } } @@ -1135,7 +1131,7 @@ func (agent *HostAgent) handleObjectDeleteForSnat(obj interface{}) { podidlist = append(podidlist, poduids...) sync = true } - delete(agent.snatPolicyLabels, objKey) + agent.DeleteSnatPolicyLabel(objKey) // Delete any Policy entries present for POD if getResourceType(obj) == POD { uid := string(obj.(*v1.Pod).ObjectMeta.UID)