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

Loki crds #3936

Merged
merged 18 commits into from
Feb 28, 2024
Merged

Loki crds #3936

merged 18 commits into from
Feb 28, 2024

Conversation

EStork09
Copy link
Contributor

PR Description

Which issue(s) this PR fixes

Fixes #3395

Notes to the Reviewer

Copied #2604 basically.
Would remove the 404 skip check when #9489 closes on the loki repo.
Had to use - to seperate rule groups/namespaces, / was not properly passing through the routes, it appears loki needs to align with mimir to handle those, loki is just using Path while mimir is using PathPrefix.

PR Checklist

  • CHANGELOG updated
  • Documentation added
  • Tests updated

@spartan0x117 spartan0x117 requested a review from a team June 23, 2023 18:59
@jcreixell jcreixell requested a review from a team June 30, 2023 15:48
Copy link
Member

@tpaschalis tpaschalis left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

This LGTM at a first glance with a few nits, but I haven't tested this yet and would like to before we check it in.

HTTPClientConfig: config.DefaultHTTPClientConfig,
}

func (args *Arguments) UnmarshalRiver(f func(interface{}) error) error {
Copy link
Member

Choose a reason for hiding this comment

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

We should make use of the new river.Validator and river. Defaulter interfaces if possible here.

85db358

Copy link
Contributor

@clayton-cornell clayton-cornell left a comment

Choose a reason for hiding this comment

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

Some suggestions for the docs

@tpaschalis
Copy link
Member

Hey @EStork09 how's it going? 👋 Would you have the time to look through the docs suggestions and rebase so we can move ahead with this?

@clayton-cornell clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Sep 18, 2023
@EStork09
Copy link
Contributor Author

EStork09 commented Dec 4, 2023

@tpaschalis sorry, I have just recently relocated to Europe, I will try to get to this when I can

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>
…pdated interface

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>
Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>
Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>
Copy link
Contributor

@clayton-cornell clayton-cornell left a comment

Choose a reason for hiding this comment

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

Small cleanup of a few agent that slipped through the first review

tpaschalis and others added 2 commits February 20, 2024 10:34
Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
Copy link
Contributor

@clayton-cornell clayton-cornell left a comment

Choose a reason for hiding this comment

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

Additional docs fixes

wildum and others added 2 commits February 27, 2024 11:18
Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
@wildum
Copy link
Contributor

wildum commented Feb 27, 2024

To avoid duplicating so much code it should be possible to put some in the common kubernetes pkg. The client part could also use a common pkg. But we can do this via a follow-up PR and focus on getting this one merged

@wildum
Copy link
Contributor

wildum commented Feb 27, 2024

I tested it and it works: I created a local k8s cluster with the agent using the helm chart and the prometheus operator. Then I created the following rule:

apiVersion: monitoring.coreos.com/v1
kind: PrometheusRule
metadata:
  creationTimestamp: null
  labels:
    prometheus: example
    role: alert-rules
  name: prometheus-example-rules
spec:
  groups:
  - name: ./example.rules
    rules:
    - alert: ExampleAlert
      expr: rate({job="test", container="my-app"} |= "error" [5m]) > 5

and with this component:

loki.rules.kubernetes "hello" {
          address = "loki-url"
          basic_auth {
              username = "username"
              password = "token"
          }
      }

It successfully pushed the alert to my grafana cloud stack:
Screenshot 2024-02-27 at 18 24 51

Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
@tpaschalis tpaschalis requested a review from wildum February 28, 2024 10:30
Copy link
Member

@tpaschalis tpaschalis left a comment

Choose a reason for hiding this comment

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

LGTM, but I haven't personally tested this so I'll leave the final word for @wildum

In any case, great work @EStork09 !

Copy link
Contributor

@wildum wildum left a comment

Choose a reason for hiding this comment

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

Made some little changes and updated with fresh main. Retested after the changes and everything works.

@wildum wildum merged commit 078149d into grafana:main Feb 28, 2024
10 checks passed
@wildum
Copy link
Contributor

wildum commented Feb 28, 2024

Thansk again @EStork09 and sorry it took so long!

@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Mar 31, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. type/docs Docs Squad label across all Grafana Labs repos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Grafana Agent Operator - Support creating rules for Loki
4 participants