[receiver/k8scluster] Add support for observing multiple namespaces #40220
Conversation
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
github-actions
bot
requested review from
ChrsMark,
dmitryax,
povilasv and
TylerHelmuth
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
|
@povilasv @TylerHelmuth @dmitryax please a look when you get the chance. |
Co-authored-by: Christos Markou <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
| if len(namespaces) == 1 { | ||
| // if there is only ony namespace in the provided namespaces array, use the deprecated field to still cover the code related to that | ||
| config.Namespace = namespaces[0] |
There was a problem hiding this comment.
Not really sure if this is needed? Why use old field?
There was a problem hiding this comment.
I left that here for now to still cover the deprecated field in the tests as well, until it is removed
There was a problem hiding this comment.
I'm not sure if we need to do it, since we want to remove it in future, no?
There was a problem hiding this comment.
yes probably, since it's going to be removed anyway I will update the PR to remove that
| return nil | ||
| var job any | ||
| var err error | ||
| exists := false | ||
|
|
||
| // first, check if there is a store for all namespaces | ||
| if jobStore, ok := jobStores[""]; ok { | ||
| job, exists, err = jobStore.GetByKey(utils.GetIDForCache(pod.Namespace, jobRef.Name)) | ||
| if err != nil { | ||
| logError(err, jobRef, pod.UID, logger) | ||
| return nil | ||
| } | ||
| } | ||
| if !exists { | ||
| // check if there is a store for the namespace of the pod | ||
| if jobStore, ok := jobStores[pod.Namespace]; ok { | ||
| job, exists, err = jobStore.GetByKey(utils.GetIDForCache(pod.Namespace, jobRef.Name)) | ||
| if err != nil { | ||
| logError(err, jobRef, pod.UID, logger) | ||
| return nil | ||
| } | ||
| } | ||
| if !exists { | ||
| logDebug(jobRef, pod.UID, logger) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Can this logic be abstracted away somehow into one method?
as we are repeating it on all stores
There was a problem hiding this comment.
i now moved that logic into the internal/utils package, along with some tests for it
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Description
This PR adapts the k8scluster receiver to support retrieving resources from multiple namespaces.
Due to the way the informer stores are maintained internally the required changes for this feature are more complex than anticipated: previously, as there was only the option to either observe all namespaces, or only one, there was one informer per GVK, whose store was used to regularly collect the metrics for the observed resources. Now, as the informers provided by the k8s library can only observe either one, or all namespaces, we can potentially have multiple informers per namespace for the same GVK. This is now done as a
map[GVK]map[string]informer.Store, where the string represents the namespace.Link to tracking issue
Fixes #40089
Testing
Added unit tests and a new e2e test
Documentation
Adapted readme to describe the added option