Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce excessive WARN logs for forbidden resource access #4356

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lou-lan
Copy link
Collaborator

@lou-lan lou-lan commented Dec 3, 2024

{"level":"WARN","ts":"2024-12-03T03:57:30.475Z","logger":"PodOwnerCache","caller":"podownercache/pod_owner_cache.go:90","msg":"","error":"error fetching owner: redisfailovers.databases.spotahome.com \"mcamel-common-redis-cluster\" is forbidden: User \"system:serviceaccount:spiderpool:spiderpool-agent\" cannot get resource \"redisfailovers\" in API group \"databases.spotahome.com\" in the namespace \"mcamel-system\""}

Optimized the PodOwnerCache logic to minimize WARN-level logs caused by forbidden access errors.
This improves log clarity by preventing unnecessary noise when the spiderpool-agent lacks permissions to access certain resources (e.g., redisfailovers in databases.spotahome.com).

Previously, errors.IsForbidden was checking the wrapped err, which led to unexpected results. The IsForbidden check has now been moved to the correct position to ensure accurate logic.

优化了 PodOwnerCache 逻辑,以最大限度地减少由 权限不足(spiderpool 用了最小权限,来收集 AI workload pod owner) 错误导致的警告级别日志(PodOwnerCache 用来缓存 pod owner 信息)。
当 spiderpool-agent 缺乏访问某些资源的权限时(例如,databases.spotahome.com 中的 redisfailovers ),这通过防止不必要的干扰来提高日志的清晰度。

原先 errors.IsForbidden 判断的是经过封装后的 err,导致结果不符合预期。现已将 IsForbidden 判断移动到正确的位置,确保逻辑准确。

@lou-lan lou-lan added release/none no release note release/bug and removed release/none no release note labels Dec 3, 2024
Copy link

codecov bot commented Dec 3, 2024

Codecov Report

Attention: Patch coverage is 45.45455% with 6 lines in your changes missing coverage. Please review.

Project coverage is 79.51%. Comparing base (2396ac0) to head (dc00d6d).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/podownercache/pod_owner_cache.go 45.45% 6 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4356      +/-   ##
==========================================
+ Coverage   79.47%   79.51%   +0.04%     
==========================================
  Files          54       54              
  Lines        6362     6362              
==========================================
+ Hits         5056     5059       +3     
+ Misses       1108     1105       -3     
  Partials      198      198              
Flag Coverage Δ
unittests 79.51% <45.45%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
pkg/podownercache/pod_owner_cache.go 78.50% <45.45%> (ø)

... and 1 file with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant