Move over to controller-runtime over doing it ourselves by davidcollom · Pull Request #348 · jetstack/version-checker · GitHub
Skip to content

Move over to controller-runtime over doing it ourselves#348

Merged
davidcollom merged 6 commits into
mainfrom
controller-runtime
Mar 31, 2025
Merged

Move over to controller-runtime over doing it ourselves#348
davidcollom merged 6 commits into
mainfrom
controller-runtime

Conversation

@davidcollom

@davidcollom davidcollom commented Mar 27, 2025

Copy link
Copy Markdown
Collaborator

During testing we identified that there was significant memory utilisation when running version-checker for 2-3 days we saw that our test cluster was running at ~400+MB when it started off at around 40-60MB.

This PR changes over from manually implemented shared informers and managing the workqueue and its goroutines, to use controller-runtime, this means we don't have to manage as much code/logic and workers ourselves and is managed for us, allowing us to focus more on the purpose of version-checker.

We do believe that there is still a memory leak, however from testing (over 24 hour period) on the same cluster, that memory usage has increased at a much slower rate, additionally we've seen that goroutines are staying at a reasonable number, where as previously it was growing exponentially at, what was an alarming rate under some situations.

@davidcollom

Copy link
Copy Markdown
Collaborator Author

Comment thread pkg/client/acr/acr.go
)

type Client struct {
*http.Client

@davidcollom davidcollom Mar 27, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

There's never a http.Client used within ACR - it creates its own - so removing.

@davidcollom

Copy link
Copy Markdown
Collaborator Author

Comment thread pkg/metrics/metrics.go
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.

3 participants