Create Github actions to validate Loki and Promtail rules #32
Conversation
| run: mkdir -p validation/recording-rules | ||
|
|
||
| - name: Generate Loki recording rules | ||
| run: yq e '.spec' loki/recording-rules.yaml > validation/recording-rules/rules.yaml |
There was a problem hiding this comment.
is this included by default in ubuntu-latest?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
is this included by default in ubuntu-latest?
It is included in Ubuntu latest. Otherwise it would not have worked
There was a problem hiding this comment.
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:
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
As an extra step; I guess we should also run po-lint (https://github.com/prometheus-operator/prometheus-operator/blob/master/Documentation/user-guides/linting.md)
I see someone made a github action for it: https://github.com/marketplace/actions/prometheus-operator-lint-action
| run: mkdir -p validation/recording-rules | ||
|
|
||
| - name: Generate Loki recording rules | ||
| run: yq e '.spec' loki/recording-rules.yaml > validation/recording-rules/rules.yaml |
There was a problem hiding this comment.
What yq invocation is this?
Generally at BitGo we assume yq is https://github.com/kislyuk/yq, rather than http://mikefarah.github.io/yq/
There was a problem hiding this comment.
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: Create tmp dir | ||
| run: mkdir -p validation/recording-rules | ||
|
|
There was a problem hiding this comment.
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: Lint rules | ||
| uses: grafana//[email protected] | ||
| env: | ||
| ACTION: lint |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
It may: e.g. if it reformats the file; then the following check will have warnings with the wrong line numbers.
This Pull Requests adds 4 Github Actions for validating the syntax of Loki and Promtail recording and alerting rules.
Also sets the ground work for adding more test in the future.
I've tested the behavior and works well, failing when there are invalid rules and succeeding when they are valid.