Bug 1882730: fix cluster:cpu_core_hyperthreading rule for s390x

Makefile

@@ -135,9 +135,17 @@ jsonnet-fmt: $(JSONNETFMT_BIN)
 shellcheck:
 	hack/shellcheck.sh
 
+tmp/rules.yaml: $(GOJSONTOYAML_BIN) assets/prometheus-k8s/rules.yaml
+	cat assets/prometheus-k8s/rules.yaml | $(GOJSONTOYAML_BIN) -yamltojson | jq .spec > "$@"
+
 .PHONY: check-rules
-check-rules: $(PROMTOOL_BIN) $(GOJSONTOYAML_BIN) assets/prometheus-k8s/rules.yaml
-	@$(PROMTOOL_BIN) check rules <(cat assets/prometheus-k8s/rules.yaml | $(GOJSONTOYAML_BIN) -yamltojson | jq .spec) | tee tmp/rules.out
+check-rules: $(PROMTOOL_BIN) tmp/rules.yaml
+	rm -f tmp/"$@".out
+	@$(PROMTOOL_BIN) check rules tmp/rules.yaml | tee "tmp/[email protected]"
+
+.PHONY: test-rules
+test-rules: check-rules
+	hack/test-rules.sh | tee "tmp/[email protected]"
 
 ###########
 # Testing #
@@ -146,8 +154,9 @@ check-rules: $(PROMTOOL_BIN) $(GOJSONTOYAML_BIN) assets/prometheus-k8s/rules.yam
 .PHONY: test
 test: test-unit test-e2e
 
+# TODO(simonpasquier): we should have a CI job specifically checking Prometheus rules.
 .PHONY: test-unit
-test-unit:
+test-unit: test-rules
 	go test -race -short $(PKGS) -count=1
 
 .PHONY: test-e2e

assets/prometheus-k8s/rules.yaml

@@ -862,11 +862,31 @@ spec:
       record: cluster:capacity_cpu_cores:sum
     - expr: |
         clamp_max(
-          (
-            label_replace( ( ( sum (node_cpu_info) by (instance, package, core) )  > 1 ), "label_node_hyperthread_enabled", "true", "instance", "(.*)" )
-            or on (instance, package)
-            label_replace( ( ( sum (node_cpu_info) by (instance, package, core) ) <= 1 ), "label_node_hyperthread_enabled", "false", "instance", "(.*)" )
-          ), 1
+          label_replace(
+            sum by(instance, package, core) (
+              node_cpu_info{core!="",package!=""}
+              or
+              # Assume core = cpu and package = 0 for platforms that don't expose core/package labels.
+              label_replace(label_join(node_cpu_info{core="",package=""}, "core", "", "cpu"), "package", "0", "package", "")
+            ) > 1,
+            "label_node_hyperthread_enabled",
+            "true",
+            "instance",
+            "(.*)"
+          ) or on (instance, package)
+          label_replace(
+            sum by(instance, package, core) (
+              label_replace(node_cpu_info{core!="",package!=""}
+              or
+              # Assume core = cpu and package = 0 for platforms that don't expose core/package labels.
+              label_join(node_cpu_info{core="",package=""}, "core", "", "cpu"), "package", "0", "package", "")
+            ) <= 1,
+            "label_node_hyperthread_enabled",
+            "false",
+            "instance",
+            "(.*)"
+          ),
+          1
         )
       record: cluster:cpu_core_hyperthreading
     - expr: |

hack/build-jsonnet.sh

@@ -4,6 +4,7 @@ set -x
 # only exit with zero if all commands of the pipeline exit successfully
 set -o pipefail
 
+# Ensure that we use the binaries from the versions defined in hack/tools/go.mod.
 PATH="$(pwd)/tmp/bin:${PATH}"
 
 prefix="assets"

hack/test-rules.sh

@@ -0,0 +1,17 @@
+#!/usr/bin/env bash
+set -e
+set -x
+# only exit with zero if all commands of the pipeline exit successfully
+set -o pipefail
+
+# Ensure that we use the binaries from the versions defined in hack/tools/go.mod.
+PATH="$(pwd)/tmp/bin:${PATH}"
+
+TMP_RULE_FILE=$(mktemp tmp/tmp.XXXXXXXXXX.yaml)
+trap 'rm -f "$TMP_RULE_FILE"' EXIT
+
+for rule in ./test/rules/*.yaml; do
+	echo ">> testing $rule <<";
+	gojsontoyaml -yamltojson - < "$rule" | jq '.rule_files=["rules.yaml"]' | gojsontoyaml - > "$TMP_RULE_FILE"
+	promtool test rules "$TMP_RULE_FILE"
+done

jsonnet/rules.jsonnet

@@ -110,11 +110,31 @@ local droppedKsmLabels = 'endpoint, instance, job, pod, service';
           {
             expr: |||
               clamp_max(
-                (
-                  label_replace( ( ( sum (node_cpu_info) by (instance, package, core) )  > 1 ), "label_node_hyperthread_enabled", "true", "instance", "(.*)" )
-                  or on (instance, package)
-                  label_replace( ( ( sum (node_cpu_info) by (instance, package, core) ) <= 1 ), "label_node_hyperthread_enabled", "false", "instance", "(.*)" )
-                ), 1
+                label_replace(
+                  sum by(instance, package, core) (
+                    node_cpu_info{core!="",package!=""}
+                    or
+                    # Assume core = cpu and package = 0 for platforms that don't expose core/package labels.
+                    label_replace(label_join(node_cpu_info{core="",package=""}, "core", "", "cpu"), "package", "0", "package", "")
+                  ) > 1,
+                  "label_node_hyperthread_enabled",
+                  "true",
+                  "instance",
+                  "(.*)"
+                ) or on (instance, package)
+                label_replace(
+                  sum by(instance, package, core) (
+                    label_replace(node_cpu_info{core!="",package!=""}
+                    or
+                    # Assume core = cpu and package = 0 for platforms that don't expose core/package labels.
+                    label_join(node_cpu_info{core="",package=""}, "core", "", "cpu"), "package", "0", "package", "")
+                  ) <= 1,
+                  "label_node_hyperthread_enabled",
+                  "false",
+                  "instance",
+                  "(.*)"
+                ),
+                1
               )
             |||,
             record: 'cluster:cpu_core_hyperthreading',

test/rules/README.md

@@ -0,0 +1,5 @@
+This directory contains [unit
+tests](https://prometheus.io/docs/prometheus/latest/configuration/unit_testing_rules/)
+for recording and alerting rules shipped by the Cluster Monitoring Operator.
+
+To execute the tests, run `make test-rules` from the root directory.