Skip to content
This repository has been archived by the owner on Oct 8, 2024. It is now read-only.

Create Github actions to validate Loki and Promtail rules #32

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

Create Github actions to validate Loki and Promtail rules #32

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 100 additions & 0 deletions .github/workflows/validate-rules-syntax.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
on: pull_request
permissions:
contents: read
pull-requests: read
jobs:
validate_loki_recording_rules:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v2

- name: Create tmp dir
run: mkdir -p validation/recording-rules

Copy link
Contributor

Choose a reason for hiding this comment

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

Consider a step like kustomize build . | kustomize cfg grep kind=PrometheusRule | kustomize cfg merge validation/recording-rules

This would not only test that the kustomization is valid; but it would apply any patches we decide to include in the repository.

- name: Generate Loki recording rules
run: yq e '.spec' loki/recording-rules.yaml > validation/recording-rules/rules.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

is this included by default in ubuntu-latest?

Copy link
Contributor

@biwwy0 biwwy0 Sep 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Can we wrap all rules in simple for loop in run: instead of creating 4 actions for different directories? Not sure how this will grow and if there will be more places where we'd add rules that need to be checked, which would require having to add them to github actions steps as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is this included by default in ubuntu-latest?

It is included in Ubuntu latest. Otherwise it would not have worked

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we wrap all rules in simple for loop in run: instead of creating 4 actions for different directories? Not sure how this will grow and if there will be more places where we'd add rules that need to be checked, which would require having to add them to github actions steps as well.

I like the idea of having separate jobs for separate files. This allows, when a job fails, to know what is broken looking at the action name. See this screenshot for an example:

image

Other issue with wrapping in into a loop, is that it will require adding more bash. I'd try to use as less bash as possible to simplify maintenance.

if there will be more places where we'd add rules that need to be checked, which would require having to add them to github actions steps as well.

Even in the for loop case, when you add a new file, you need to add the filename to the for loop.
Unless we add even more logic for finding files with rules. That sounds like a lot of bash and complexity unneeded.

Anyway, I think you point in a valid one. Let's see what others think and reach consensus. I'm open to modify this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I'd like the one step so that we can make this cover all rules automatically.

However I agree with making what failed more obvious.
How about the idea at https://github.com/grafana/cortex-rules-action#pull-request-diff where it comments on the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok 2 against 1, we have majority.
I'm going to test how to get all rules tested in one step in an elegant way. I'll give a try to the comments in the PR too.

Copy link
Contributor

Choose a reason for hiding this comment

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

What yq invocation is this?
Generally at BitGo we assume yq is https://github.com/kislyuk/yq, rather than http://mikefarah.github.io/yq/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It took my some time to figure out that the yq included in Ubuntu is coming from http://mikefarah.github.io/yq/ so it has a slightly different syntax.

I'd favor using a different yq implementation which comes already with Ubuntu rather than installing another package for simplicity.


- name: Lint rules
uses: grafana//[email protected]
env:
ACTION: lint
Copy link
Contributor

Choose a reason for hiding this comment

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

lint

Lints a rules file(s). The linter's aim is not to verify correctness but to fix YAML and PromQL expression formatting within the rule file(s). The linting happens in-place within the specified file(s). Does not interact with your Cortex cluster.

I'm not sure linting here makes sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

However it won't hurt IMO.

Copy link
Contributor

Choose a reason for hiding this comment

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

It may: e.g. if it reformats the file; then the following check will have warnings with the wrong line numbers.

RULES_DIR: validation/recording-rules/

- name: Check rules
uses: grafana//[email protected]
env:
ACTION: check
RULES_DIR: validation/recording-rules/

validate_loki_alerting_rules:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v2

- name: Create tmp dir
run: mkdir -p validation/alerting-rules

- name: Generate Loki alerting rules
run: yq e '.spec' loki/alerting-rules.yaml > validation/alerting-rules/rules.yaml

- name: Lint rules
uses: grafana//[email protected]
env:
ACTION: lint
RULES_DIR: validation/alerting-rules/

- name: Check rules
uses: grafana//[email protected]
env:
ACTION: check
RULES_DIR: validation/alerting-rules/

validate_promtail_recording_rules:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v2

- name: Create tmp dir
run: mkdir -p validation/recording-rules

- name: Generate Loki recording rules
run: yq e '.spec' promtail/recording-rules.yaml > validation/recording-rules/rules.yaml

- name: Lint rules
uses: grafana//[email protected]
env:
ACTION: lint
RULES_DIR: validation/recording-rules/

- name: Check rules
uses: grafana//[email protected]
env:
ACTION: check
RULES_DIR: validation/recording-rules/

validate_promtail_alerting_rules:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v2

- name: Create tmp dir
run: mkdir -p validation/alerting-rules

- name: Generate Loki alerting rules
run: yq e '.spec' promtail/alerting-rules.yaml > validation/alerting-rules/rules.yaml

- name: Lint rules
uses: grafana//[email protected]
env:
ACTION: lint
RULES_DIR: validation/alerting-rules/

- name: Check rules
uses: grafana//[email protected]
env:
ACTION: check
RULES_DIR: validation/alerting-rules/