-
Notifications
You must be signed in to change notification settings - Fork 25
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
NETOBSERV-101 - loki statusURL #154
NETOBSERV-101 - loki statusURL #154
Conversation
58258ce
to
25f3820
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
almost lgtm, just a typo + small remark
api/v1alpha1/flowcollector_types.go
Outdated
//+kubebuilder:validation:optional | ||
// StatusURL specifies the address of the Loki /ready /metrics /config endpoints, in case it is different from the | ||
// Loki querier URL. If empty, the QuerierURL value will be used. | ||
// This is usefull to show error messages and some context in the frontend |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: useful
api/v1alpha1/flowcollector_types.go
Outdated
@@ -300,9 +300,15 @@ type FlowCollectorLoki struct { | |||
//+kubebuilder:validation:optional | |||
// QuerierURL specifies the address of the Loki querier service, in case it is different from the | |||
// Loki ingester URL. If empty, the URL value will be used (assuming that the Loki ingester | |||
// and querier are int he same host). | |||
// and querier are in the same host). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpicking, but "same host" is not totally correct IMO, I would rephrase as "same server", no? (they can be on same host (node), but different servers / deployments)
btw: what would be the URL to set when using the loki-operator with the gateway? is it documented? |
You need to point query frontend in that case. Check https://github.com/netobserv/documents/pull/22/files#diff-7f51acaa5430f57ab824677d12e76ad94983d7dd45c949b7cb1335991a019996R97 |
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jpinsonneau The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
BUILD: add fast flag to lint
Following observatorium/api#350 we will not have access to loki ready, config, metrics and build infos endpoints using the gateway.
This PR allow to configure a
statusURL
in the CRD and send it to the console pluginRelated PR: netobserv/network-observability-console-plugin#186