Skip to content

Commit 89d3d9d

Browse files
giuliano-siderbruegth
authored andcommitted
[processor/k8s_attributes] Fix memory leak in k8s_attributes map (open-telemetry#48987)
[processor/k8s_attributes] Fix memory leak in k8s_attributes map due to missing IP or other dynamic attributes from Pod deletion event object. #### Component(s) [processor/k8s_attributes] #### Description In environments with high pod churn or rapid scale-down, the `k8sattributes` processor can leak pod IP-based cache entries (`connection: <IP>` and `resource_attribute: k8s.pod.ip`) in the internal `c.Pods` map. This leads to unbounded memory growth and a stable baseline of leaked keys even after the actual pods have been scaled down to zero. #### Root Cause When a pod is deleted, the `WatchClient`'s [`forgetPod`](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/processor/k8sattributesprocessor/internal/kube/client.go#L1635) method is called to purge the cached entries: ```go func (c *WatchClient) forgetPod(pod *api_v1.Pod) { podToRemove := c.podFromAPI(pod) identifiers := c.getIdentifiersFromAssoc(podToRemove) for i := range identifiers { id := identifiers[i] p, ok := c.GetPod(id) if ok && p.PodUID == string(pod.UID) { c.appendDeleteQueue(id, p.PodUID) } } } ``` 1. `forgetPod` builds `podToRemove` from the incoming delete event payload `pod`. 2. It then calls `getIdentifiersFromAssoc(podToRemove)` to determine which keys to add to the delete queue. 3. If the CNI has already reclaimed/cleared the Pod's IP address by the time the final `DELETE` event is dispatched (or if the status payload is incomplete), `pod.Status.PodIP` will be empty (`""`). 4. As a result, `getIdentifiersFromAssoc` **only** generates the UID-based identifier (`resource_attribute: k8s.pod.uid`). The IP-based identifiers (`connection: <IP>` and `resource_attribute: k8s.pod.ip`) are skipped. 5. Consequently, only the UID-based key is queued for deletion, while the IP-based keys are never cleared and are permanently leaked in the `c.Pods` map. #### Proposed Solution Instead of relying on the incoming delete event status to determine the keys to delete, `forgetPod` should look up the cached pod from `c.Pods` using the pod's UID (which is always present in the event). The cached pod object is guaranteed to contain the IP address and all attributes as they were stored during the pod's lifecycle. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#48986 <!--Describe what testing was performed and which tests were added.--> #### Testing Added a unit test case that catches the issue of a Pod object from a deletion event that is missing IP fields. <!--Describe the documentation added.--> #### Documentation Added a changelog entry for a bug fix. <!--Authorship attestation. See AGENTS.md for details. AI agents must not check this box on behalf of the user; the human author must check it themselves before the PR is ready for review.--> #### Authorship - [ X ] I, a human, wrote this pull request description myself. <!--Please delete paragraphs that you did not use before submitting.-->
1 parent f52800a commit 89d3d9d

3 files changed

Lines changed: 85 additions & 2 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
2+
change_type: bug_fix
3+
4+
# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog)
5+
component: processor/k8s_attributes
6+
7+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
8+
note: "Fix cache key memory leak in k8sattributesprocessor when a Pod's IP is missing or cleared from the delete event"
9+
10+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
11+
issues: [48986]
12+
13+
# (Optional) One or more lines of additional information to render under the primary note.
14+
subtext: |
15+
When a Pod is deleted, if its IP address is missing or already cleared from the delete event status,
16+
the processor now looks up the cached pod by its UID to retrieve the stored IP and correctly queue
17+
all associated keys for deletion.
18+
19+
change_logs: [user]

processor/k8sattributesprocessor/internal/kube/client.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1655,8 +1655,27 @@ func (c *WatchClient) addOrUpdatePod(pod *api_v1.Pod) {
16551655
}
16561656

16571657
func (c *WatchClient) forgetPod(pod *api_v1.Pod) {
1658-
podToRemove := c.podFromAPI(pod)
1659-
identifiers := c.getIdentifiersFromAssoc(podToRemove)
1658+
// Look up the cached pod using its UID. Unlike dynamic status attributes (such as IP addresses)
1659+
// which may be cleared or missing in the final DELETE event payload, the Pod UID is immutable
1660+
// and guaranteed to be present. Using the cached pod ensures we generate and clean up all keys
1661+
// under which the pod was originally registered.
1662+
uidKey := PodIdentifier{
1663+
PodIdentifierAttributeFromResourceAttribute(string(conventions.K8SPodUIDKey), string(pod.UID)),
1664+
}
1665+
c.m.RLock()
1666+
cachedPod, ok := c.Pods[uidKey]
1667+
c.m.RUnlock()
1668+
1669+
var identifiers []PodIdentifier
1670+
if ok {
1671+
identifiers = c.getIdentifiersFromAssoc(cachedPod)
1672+
} else {
1673+
// Fallback: if the pod was never added to the cache (e.g. startup/informer sync edge cases),
1674+
// generate deletion keys from the incoming delete event payload directly.
1675+
podToRemove := c.podFromAPI(pod)
1676+
identifiers = c.getIdentifiersFromAssoc(podToRemove)
1677+
}
1678+
16601679
for i := range identifiers {
16611680
id := identifiers[i]
16621681
p, ok := c.GetPod(id)

processor/k8sattributesprocessor/internal/kube/client_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4874,3 +4874,48 @@ func TestExtractPodAttributesClusterUIDRace(t *testing.T) {
48744874

48754875
wg.Wait()
48764876
}
4877+
4878+
func TestPodDeleteIPMissingFromDeleteEvent(t *testing.T) {
4879+
c, _ := newTestClient(t)
4880+
4881+
pod := &api_v1.Pod{}
4882+
pod.Name = "podLeak"
4883+
pod.Status.PodIP = "4.4.4.4"
4884+
pod.UID = "uid-leak-test"
4885+
c.handlePodAdd(pod)
4886+
4887+
// Map should have 3 keys for this pod
4888+
assert.Contains(t, c.Pods, newPodIdentifier("resource_attribute", "k8s.pod.uid", "uid-leak-test"))
4889+
assert.Contains(t, c.Pods, newPodIdentifier("connection", "k8s.pod.ip", "4.4.4.4"))
4890+
assert.Contains(t, c.Pods, newPodIdentifier("resource_attribute", "k8s.pod.ip", "4.4.4.4"))
4891+
4892+
// Clear the delete queue
4893+
c.deleteQueue = c.deleteQueue[:0]
4894+
4895+
// Simulate delete event where PodIP is missing/empty!
4896+
deletePod := &api_v1.Pod{}
4897+
deletePod.Name = "podLeak"
4898+
deletePod.UID = "uid-leak-test"
4899+
deletePod.Status.PodIP = ""
4900+
4901+
c.handlePodDelete(deletePod)
4902+
4903+
// In the bug state, only the UID-based keys are queued for deletion, leaving the IP-based keys leaked.
4904+
// We expect all unique key patterns (UID, connection IP, and Pod IP) to be queued for deletion.
4905+
var ids []PodIdentifier
4906+
for _, req := range c.deleteQueue {
4907+
assert.Equal(t, "uid-leak-test", req.podUID)
4908+
ids = append(ids, req.id)
4909+
}
4910+
4911+
// Verify that each of the three key patterns was queued at least once
4912+
assert.Contains(t, ids, newPodIdentifier("resource_attribute", "k8s.pod.uid", "uid-leak-test"))
4913+
assert.Contains(t, ids, newPodIdentifier("connection", "k8s.pod.ip", "4.4.4.4"))
4914+
assert.Contains(t, ids, newPodIdentifier("resource_attribute", "k8s.pod.ip", "4.4.4.4"))
4915+
4916+
// Run the sweep logic to process all queued deletions immediately
4917+
c.deleteLoopProcessing(0)
4918+
4919+
// In the fixed state, all keys associated with the pod should be successfully cleaned up from the cache.
4920+
assert.Empty(t, c.Pods)
4921+
}

0 commit comments

Comments
 (0)