refactor: persist collectors memory settings in collectorsgroup CRD #1824
Conversation
This file contains 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
RonFed
approved these changes
Nov 22, 2024
| // - running out of memory and being killed by the k8s OOM killer | ||
| // - consuming all available memory on the node which can lead to node instability | ||
| // - pushing back pressure to the instrumented applications | ||
| MemorySettings CollectorsGroupMemorySettings `json:"memorySettings"` |
There was a problem hiding this comment.
Should this have omitempty or default values in the kubebuilder level?
I saw that currently the default values are defined in the code. Perhaps defining them in the schema will be better?
There was a problem hiding this comment.
Should this have omitempty or default values in the kubebuilder level?
that's intentional, since the consumers just use this value as is so it must be set
I saw that currently the default values are defined in the code. Perhaps defining them in the schema will be better?
I agree, since my focus now is elsewhere, I prefer to do this refactor and leave the current behavior as is. might be addressed in followup PRs
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR makes it so:
It aim to simplify the reconciliation processes in autoscaler, and make it depend on one less resource. any config computations are offloaded to scheduler and should trigger autoscaler less.
This PR is pure refactor and does not change any existing behavior. follow up PRs will introduce changes, additions, and applying memory settings also in node collector.