fix: issues with prometheus kubernetes pod discovery#9605
Merged
reimda merged 3 commits intoinfluxdata:masterfrom Aug 17, 2021
Merged
fix: issues with prometheus kubernetes pod discovery#9605reimda merged 3 commits intoinfluxdata:masterfrom
reimda merged 3 commits intoinfluxdata:masterfrom
Conversation
Contributor
Looks like new artifacts were built from this PR. Get them here!Artifact URLs |
sspaink
approved these changes
Aug 10, 2021
Contributor
sspaink
left a comment
There was a problem hiding this comment.
lgtm! Thank you for fixing this and describing the why/how so clearly.
reimda
approved these changes
Aug 17, 2021
Contributor
reimda
left a comment
There was a problem hiding this comment.
Looks reasonable to me. I don't have a deep understanding of k8s.io/client-go/kubernetes but it sounds like you have identified valid problems and provided good fixes.
3 tasks
reimda
pushed a commit
that referenced
this pull request
Aug 19, 2021
(cherry picked from commit fe144e7)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Required for all PRs:
Resolves #9600
Get the pod info from the event using
event.Object.(*corev1.Pod)so that the pod isn't an empty struct. This is the equivalent ofwatcher.Next(pod)when the previous client package was used.A go routine is already running that keeps calling the watchPod() function whenever it returns. Right now with the watchPod() function is calling its own go routine, the function returns and watchPod is called again. This continuously creates new threads and new watchers and re-registers pods instead of having one watcher running each time. Changed back to the previous functionality with a for loop that returns when
ctx.Done()Added
defer watcher.Stop()which is the equivalent ofdefer watcher.Close()when the previous client package was used.I'm not sure any unit tests can be added to detect these issues since the function relies on the kubernetes watch api call and discovering kubernetes pods.