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

Allow skipping per-query namespace inference via config #24

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

Conversation

jotak
Copy link
Contributor

@jotak jotak commented Jan 11, 2024

This is a way to open the current behaviour of Loki "meta-data" queries to other use cases where the list of namespaces cannot be inferred from queries.

NetObserv will use this flag because its queries are not namespaced. This allows to effectively use the fine-grained SAR feature with netobserv.

Here's a diagram summarizing the workflow related to namespaced SARs. Note that this PR doesn't change the diagram flows; it's only adding a use case to one of the branching, mentioned as "typical netobserv query"


 ┌───────────────────────────────────┐       ┌─────────────────────────────────────────────────────────┐
 │                                   │       │                                                         │
 │ Namespace(s) can be inferred      │       │ Namespace(s) can't be inferred                          │
 │ from query                        │       │ from query                                              │
 │ E.g: query/q=                     │       │ E.g:                                                    │
 │ {namespace="myns"} | pod:"my-pod" │       │ - label/key/values (metadata query)                     │
 │                                   │       │ - query/q={src_node="node-1"} (typical netobserv query) │
 └────────────────┬──────────────────┘       │                                                         │
                  │                          └─────────────────────────┬───────────────────────────────┘
                  │                                                    │
                  │                                                    │
       ┌──────────▼───────────┐                        ┌───────────────▼───────────────┐
       │                      │                        │                               │
       │ Extract namespace(s) │                        │ Get all accessible namespaces │
       │                      │                        │                               │
       └───────────┬──────────┘                        └───────────────┬───────────────┘
                   │                                                   │
                   │                                                   │
                   │                                                   │
                   │                                                   │
                   │             ┌────────────────────────┐            │
                   └─────────────►                        ◄────────────┘
                                 │ SAR for each namespace │
                                 │                        │
                                 └───────────┬────────────┘
                                             │
                                             │
                                             │
                            ┌────────────────▼───────────────────┐
                            │                                    │
                            │ Inject allowed namespaces in query │
                            │                                    │
                            └────────────────────────────────────┘

This is a way to open the current behaviour of Loki "meta-data"
queries to other use cases where the list of namespaces cannot be
inferred from queries.

NetObserv will use this flag because its queries are not namespaced.
This allows to effectively use the fine-grained SAR feature with
netobserv.

Related JIRA: NETOBSERV-1324
@jotak
Copy link
Contributor Author

jotak commented Jan 15, 2024

@periklis I'm testing and monitoring the impact on Kube API, what I can already tell is that the impact is largely contained by the QPS and Burst settings of the kube client, which by default lead to just 5 queries per second: that's very conservative and protective for the kube API and on the other hand it makes the SAR queries very long to answer when there are a lot of namespaces to check. For instance, 100 allowed namespaces lead to 20 seconds (minimum) for the query. Results are cached per gateway instance, which mitigates this slow response problem (next calls are much faster).

With this protective QPS, I didn't notice any negative impact on the kube API; for instance, check for reponse time metrics, they're very stable, looking not impacted by the many SAR queries.

@jotak
Copy link
Contributor Author

jotak commented Jan 15, 2024

Tested with 100 SARs and a QPS/Burst of 50 makes the queries much faster (10x faster) and still no noticeable impact on the kube API performance:

Expected increase in queries per second:
Capture d’écran du 2024-01-15 14-13-59

No visible impact on response time:
Capture d’écran du 2024-01-15 14-14-28

Note that the QPS / Burst client settings are shared across users (per gateway instance), so I don't think having multiple concurrent users would have much more impact ...

@xperimental
Copy link
Collaborator

@jotak Sorry, I did not get to this yet, but it is on my list. I want to first test this on a cluster myself, so that I can understand what you're trying to achieve. I hope I'll make it this week.

@jotak
Copy link
Contributor Author

jotak commented Jan 18, 2024

thanks @xperimental - I can help you to set up netobserv if you want: you would see how it enables fine-grained rbac for us

@jotak
Copy link
Contributor Author

jotak commented May 16, 2024

Hey @xperimental, @periklis : this has been waiting for a very long time, any chance you could look at it?

jotak added a commit to jotak/loki that referenced this pull request May 21, 2024
In Loki operator, when set up with openshift for network logs, pass the
--opa.skip-namespace-inference flag set to true to allow fine-grained
RBAC

Related: observatorium/opa-openshift#24

Signed-off-by: Joel Takvorian <jtakvori@redhat.com>
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.

None yet

2 participants