Skip to content

event-exporter: fix OOM issue, conditionally enable WatchListClient for pod-owner-label#1084

Open
zxqlxy wants to merge 5 commits intoGoogleCloudPlatform:masterfrom
zxqlxy:oom-fix
Open

event-exporter: fix OOM issue, conditionally enable WatchListClient for pod-owner-label#1084
zxqlxy wants to merge 5 commits intoGoogleCloudPlatform:masterfrom
zxqlxy:oom-fix

Conversation

@zxqlxy
Copy link

@zxqlxy zxqlxy commented Mar 7, 2026

WatchListClient should fix the OOM issue. Will default to WatchListClient later for watcher.

Xinyun Liu and others added 3 commits March 6, 2026 23:55
@zxqlxy zxqlxy force-pushed the oom-fix branch 2 times, most recently from 783c0d9 to 92d72f8 Compare March 7, 2026 09:34
Copy link
Contributor

@erain erain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two blocking issues here:

  1. event-exporter/.gitignore now ignores vendor/**. This repo commits event-exporter/vendor, so that pattern causes newly added vendored packages to stay ignored and never make it into the PR.

  2. As submitted, the supported -mod=vendor build path is broken. event-exporter/Makefile and event-exporter/Dockerfile both build/test with -mod=vendor, but the PR adds dependencies/imports that are referenced in vendor/modules.txt without the corresponding source trees actually being present in event-exporter/vendor.

I verified this by running go test -mod=vendor ./... in event-exporter, which fails on this branch with missing packages such as k8s.io/client-go/metadata and sigs.k8s.io/structured-merge-diff/v6/value. After rerunning go mod vendor, the same command passes. Please regenerate and commit the full vendor tree and drop the vendor/** ignore.

@@ -1,2 +1,3 @@
event-exporter
.idea/**
vendor/** No newline at end of file
Copy link
Contributor

@erain erain Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vendor/** ignore is blocking the vendor refresh required by this change. Since this repo checks in event-exporter/vendor, this pattern causes newly added dependency directories to stay ignored and never make it into the PR.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must have added to make the file change clear when debugging, fixed now

@zxqlxy zxqlxy requested a review from erain March 9, 2026 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants