Skip to content

Add PrometheusRule to com.coreos CRD - #45

Merged
porridge merged 2 commits into
mainfrom
add-prometheus-rule-CRD
Aug 2, 2023
Merged

Add PrometheusRule to com.coreos CRD#45
porridge merged 2 commits into
mainfrom
add-prometheus-rule-CRD

Conversation

@stehessel

Copy link
Copy Markdown
Contributor

I want to test a PrometheusRule CR, hence I need it to be available in the CRD. The diff within com.coreos.json.gz is:

> "com.coreos.monitoring.v1.PrometheusRule": {
> "description": "PrometheusRule defines recording and alerting rules for a Prometheus instance",
> "properties": {
> "apiVersion": {
> "description": "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources",
> "type": "string"
> },
> "kind": {
> "description": "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requeststo. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds",
> "type": "string"
> },
> "metadata": {
> "type": "object"
> },
> "spec": {
> "description": "Specification of desired alerting rule definitions for Prometheus.",
> "properties": {
> "groups": {
> "description": "Content of Prometheus rule file",
> "items": {
> "description": "RuleGroup is a list of sequentially evaluated recording and alerting rules.",
> "properties": {
> "interval": {
> "description": "Interval determines how often rules in the group are evaluated.",
> "pattern": "^(0|(([0-9]+)y)?(([0-9]+)w)?(([0-9]+)d)?(([0-9]+)h)?(([0-9]+)m)?(([0-9]+)s)?(([0-9]+)ms)?)$",
> "type": "string"
> },
> "name": {
> "description": "Name of the rule group.",
> "minLength": 1,
> "type": "string"
> },
> "partial_response_strategy": {
> "description": "PartialResponseStrategy is only used by ThanosRuler and will be ignored by Prometheus instances. More info: https://github.com/thanos-io/thanos/blob/main/docs/components/rule.md#partial-response",
> "pattern": "^(?i)(abort|warn)?$",
> "type": "string"
> },
> "rules": {
> "description": "List of alerting and recording rules.",
> "items": {
> "description": "Rule describes an alerting or recording rule See Prometheus documentation: [alerting](https://www.prometheus.io/docs/prometheus/latest/configuration/alerting_rules/) or [recording](https://www.prometheus.io/docs/prometheus/latest/configuration/recording_rules/#recording-rules) rule",
> "properties": {
> "alert": {
> "description": "Name of the alert. Must be a valid label value. Only one of `record` and `alert` must be set.",
> "type": "string"
> },
> "annotations": {
> "additionalProperties": {
> "type": "string"
> },
> "description": "Annotations to add to each alert. Only valid for alerting rules.",
> "type": "object"
> },
> "expr": {
> "anyOf": [
> {
> "type": "integer"
> },
> {
> "type": "string"
> }
> ],
> "description": "PromQL expression to evaluate.",
> "x-kubernetes-int-or-string": true
> },
> "for": {
> "description": "Alerts are considered firing once they have been returned for this long.",
> "pattern": "^(0|(([0-9]+)y)?(([0-9]+)w)?(([0-9]+)d)?(([0-9]+)h)?(([0-9]+)m)?(([0-9]+)s)?(([0-9]+)ms)?)$",
> "type": "string"
> },
> "labels": {
> "additionalProperties": {
> "type": "string"
> },
> "description": "Labels to add or overwrite.",
> "type": "object"
> },
> "record": {
> "description": "Name of the time series to output to. Must be a valid metric name. Only one of `record` and `alert` must be set.",
> "type": "string"
> }
> },
> "required": ["expr"],
> "type": "object"
> },
> "type": "array"
> }
> },
> "required": ["name", "rules"],
> "type": "object"
> },
> "type": "array",
> "x-kubernetes-list-map-keys": ["name"],
> "x-kubernetes-list-type": "map"
> }
> },
> "type": "object"
> }
> },
> "required": ["spec"],
> "type": "object",
> "x-kubernetes-group-version-kind": [
> {
> "group": "monitoring.coreos.com",
> "kind": "PrometheusRule",
> "version": "v1"
> }
> ]

@stehessel
stehessel requested a review from a teamAugust 1, 2023 01:05
@stehessel
stehesselforce-pushed the add-prometheus-rule-CRD branch from 7103fc1 to 4641bb7CompareAugust 1, 2023 11:03
@stehessel
stehesselforce-pushed the add-prometheus-rule-CRD branch from 4641bb7 to ff34595CompareAugust 1, 2023 11:45
@porridge

Copy link
Copy Markdown
Contributor
 Error: Received unexpected error:
[945,33] $root.definitions.com.coreos.monitoring.v1.PrometheusRule.properties.spec.properties.groups.items.<array>.properties.rules.items.<array>.properties.expr has invalid property: anyOf

Any ideas @stehessel ?

@stehessel

Copy link
Copy Markdown
ContributorAuthor

How is that CRD validated? I did a oc get crd prometheusrules.monitoring.coreos.com -o json | jq '.spec.versions[0].schema.openAPIV3Schema' on my OpenShift 4.13 cluster. Maybe the jsonschema version used by this package is too old for that?

@stehessel

Copy link
Copy Markdown
ContributorAuthor

@porridge Indeed it seems that helmtest is using OpenAPI v2, however anyof support was added in v3. Do you think we could update to v3 to get extended jsonschema support?

@stehessel

stehessel commented Aug 1, 2023

Copy link
Copy Markdown
ContributorAuthor

I replaced the anyOf with a typeless schema to make it OpenAPI v2 compatible.

"expr": {
"anyOf": [
{
"type": "integer"
},
{
"type": "string"
}
],
"description": "PromQL expression to evaluate.",
"x-kubernetes-int-or-string": true
},

changed to

"expr": {},

@porridgeporridge mentioned this pull request Aug 2, 2023

@porridgeporridge left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess this workaround is OK for now. Filed #46 for visibility.

@porridge
porridge merged commit 1732d1a into mainAug 2, 2023
@porridge
porridge deleted the add-prometheus-rule-CRD branch August 2, 2023 05:18
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@stehessel@porridge