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

chore: add pint #958

Merged
merged 1 commit into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ jobs:
run: make --always-make vale && git diff --exit-code
- name: Jsonnet linter
run: make --always-make jsonnet-lint
- name: YAML linter (Pint)
run: make --always-make pint-lint
- name: Unit tests
run: make --always-make test

Expand Down
15 changes: 12 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ MD_FILES = $(shell find . \( -type d -name '.vale' -o -type d -name 'vendor' \)
MARKDOWNFMT_BIN=$(BIN_DIR)/markdownfmt
VALE_BIN=$(BIN_DIR)/vale
PROMTOOL_BIN=$(BIN_DIR)/promtool
TOOLING=$(JB_BIN) $(JSONNETLINT_BIN) $(JSONNET_BIN) $(JSONNETFMT_BIN) $(PROMTOOL_BIN) $(GRAFANA_DASHBOARD_LINTER_BIN) $(MARKDOWNFMT_BIN) $(VALE_BIN)
PINT_BIN=$(BIN_DIR)/pint
TOOLING=$(JB_BIN) $(JSONNETLINT_BIN) $(JSONNET_BIN) $(JSONNETFMT_BIN) $(PROMTOOL_BIN) $(GRAFANA_DASHBOARD_LINTER_BIN) $(MARKDOWNFMT_BIN) $(VALE_BIN) $(PINT_BIN)
JSONNETFMT_ARGS=-n 2 --max-blank-lines 2 --string-style s --comment-style s
SRC_DIR ?=dashboards
OUT_DIR ?=dashboards_out
Expand Down Expand Up @@ -44,7 +45,7 @@ $(OUT_DIR): $(JSONNET_BIN) $(JSONNET_VENDOR) mixin.libsonnet lib/dashboards.json
@$(JSONNET_BIN) -J vendor -m $(OUT_DIR) lib/dashboards.jsonnet

.PHONY: lint
lint: jsonnet-lint alerts-lint dashboards-lint vale
lint: jsonnet-lint alerts-lint dashboards-lint vale pint-lint

.PHONY: jsonnet-lint
jsonnet-lint: $(JSONNETLINT_BIN) $(JSONNET_VENDOR)
Expand All @@ -67,10 +68,18 @@ dashboards-lint: $(GRAFANA_DASHBOARD_LINTER_BIN) $(OUT_DIR)/.lint

.PHONY: vale
vale: $(VALE_BIN)
@echo $(MD_FILES)
@$(VALE_BIN) sync && \
$(VALE_BIN) $(MD_FILES)

.PHONY: pint-lint
pint-lint: generate $(PINT_BIN)
@# Pint will not exit with a non-zero status code if there are linting issues.
@output=$$($(PINT_BIN) -n -o -l WARN lint prometheus_alerts.yaml prometheus_rules.yaml 2>&1); \
if [ -n "$$output" ]; then \
echo "\n$$output"; \
exit 1; \
fi

.PHONY: clean
clean:
# Remove all files and directories ignored by git.
Expand Down
2 changes: 1 addition & 1 deletion alerts/apps_alerts.libsonnet
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@
{
expr: |||
(
max without (revision) (
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder why this change?

Copy link
Collaborator Author

@rexagod rexagod Jul 18, 2024

Choose a reason for hiding this comment

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

Pint suggested a refactor here.

┌[rexagod@nebuchadnezzar] [/dev/ttys000] [pint ⚡] [2]
└[~/repositories/oss/kubernetes-mixin]> make pint-lint 

prometheus_alerts.yaml:106-123 Warning: Aggregation using `without()` can be fragile when used inside binary expression because both sides must have identical sets of labels to produce any results, adding or removing labels to metrics used here can easily break the query, consider aggregating using `by()` to ensure consistent labels. (promql/fragile)
 106 |     "expr": |
 107 |       (
 108 |         max without (revision) (
 109 |           kube_statefulset_status_current_revision{job="kube-state-metrics"}
 110 |             unless
 111 |           kube_statefulset_status_update_revision{job="kube-state-metrics"}
 112 |         )
 113 |           *
 114 |         (
 115 |           kube_statefulset_replicas{job="kube-state-metrics"}
 116 |             !=
 117 |           kube_statefulset_status_replicas_updated{job="kube-state-metrics"}
 118 |         )
 119 |       )  and (
 120 |         changes(kube_statefulset_status_replicas_updated{job="kube-state-metrics"}[5m])
 121 |           ==
 122 |         0
 123 |       )
make: *** [pint-lint] Error 1

max by(namespace, statefulset) (
kube_statefulset_status_current_revision{%(prefixedNamespaceSelector)s%(kubeStateMetricsSelector)s}
unless
kube_statefulset_status_update_revision{%(prefixedNamespaceSelector)s%(kubeStateMetricsSelector)s}
Expand Down
Loading