Skip to content
Open
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

Large diffs are not rendered by default.

137 changes: 137 additions & 0 deletions config/v1alpha1/types_cluster_monitoring.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,12 @@ type ClusterMonitoringSpec struct {
// When set, at least one field must be specified within monitoringPluginConfig.
// +optional
MonitoringPluginConfig MonitoringPluginConfig `json:"monitoringPluginConfig,omitempty,omitzero"`
// kubeStateMetricsConfig is an optional field that can be used to configure the kube-state-metrics
// agent that runs in the openshift-monitoring namespace. kube-state-metrics generates metrics about
// the state of Kubernetes objects such as Deployments, Nodes, and Pods.
// When omitted, this means no opinion and the platform is left to choose a reasonable default, which is subject to change over time.
// +optional
KubeStateMetricsConfig KubeStateMetricsConfig `json:"kubeStateMetricsConfig,omitempty,omitzero"`
}

// OpenShiftStateMetricsConfig provides configuration options for the openshift-state-metrics agent
Expand Down Expand Up @@ -2510,3 +2516,134 @@ type Audit struct {
// +required
Profile AuditProfile `json:"profile,omitempty"`
}

// KubeStateMetricsConfig provides configuration options for the kube-state-metrics agent
// that runs in the `openshift-monitoring` namespace. kube-state-metrics generates metrics
// about the state of Kubernetes objects such as Deployments, Nodes, and Pods.
// +kubebuilder:validation:MinProperties=1
type KubeStateMetricsConfig struct {
// nodeSelector defines the nodes on which the Pods are scheduled.
// nodeSelector is optional.
//
// When omitted, this means the user has no opinion and the platform is left
// to choose reasonable defaults. These defaults are subject to change over time.
// The current default value is `kubernetes.io/os: linux`.
// When specified, nodeSelector must contain at least 1 entry and must not contain more than 10 entries.
// +optional
// +kubebuilder:validation:MinProperties=1
// +kubebuilder:validation:MaxProperties=10
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
// resources defines the compute resource requests and limits for the kube-state-metrics container.
// This includes CPU, memory and HugePages constraints to help control scheduling and resource usage.
// When not specified, defaults are used by the platform. Requests cannot exceed limits.
// This field is optional.
// More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
// This is a simplified API that maps to Kubernetes ResourceRequirements.
// The current default values are:
// resources:
// - name: cpu
// request: 4m
// - name: memory
// request: 40Mi
// Maximum length for this list is 5.
// Minimum length for this list is 1.
// Each resource name must be unique within this list.
// +optional
// +listType=map
// +listMapKey=name
// +kubebuilder:validation:MaxItems=5
// +kubebuilder:validation:MinItems=1
Resources []ContainerResource `json:"resources,omitempty"`
// tolerations defines tolerations for the pods.
// tolerations is optional.
//
// When omitted, no tolerations are applied. This default is subject to change over time.
// When specified, tolerations must contain at least 1 entry and must not contain more than 10 entries.
// +kubebuilder:validation:MaxItems=10
// +kubebuilder:validation:MinItems=1
// +listType=atomic
// +optional
Tolerations []v1.Toleration `json:"tolerations,omitempty"`
// topologySpreadConstraints defines rules for how kube-state-metrics Pods should be distributed
// across topology domains such as zones, nodes, or other user-defined labels.
// topologySpreadConstraints is optional.
// This helps improve high availability and resource efficiency by avoiding placing
// too many replicas in the same failure domain.
//
// This field maps directly to the `topologySpreadConstraints` field in the Pod spec.
// When omitted, no topology spread constraints are applied. This default is subject to change over time.
// When specified, topologySpreadConstraints must contain at least 1 entry and must not contain more than 10 entries.
Comment on lines +2573 to +2575
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same issue as tolerations: "defaults may change over time" followed by "Defaults are empty/unset" is contradictory. Consider:

"When omitted, no topology spread constraints are applied. This default is subject to change over time."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed as well!

// Entries must have unique topologyKey and whenUnsatisfiable pairs.
// +kubebuilder:validation:MaxItems=10
// +kubebuilder:validation:MinItems=1
// +listType=map
// +listMapKey=topologyKey
// +listMapKey=whenUnsatisfiable
// +optional
TopologySpreadConstraints []v1.TopologySpreadConstraint `json:"topologySpreadConstraints,omitempty"`
// additionalResourceLabels defines additional Kubernetes resource labels to expose as metrics
// in kube-state-metrics, in addition to the default set.
// Currently, only "Jobs" and "CronJobs" resources are supported due to cardinality concerns.
// Each entry specifies a resource name and a list of Kubernetes label names to expose.
// Use "*" in the labels list to expose all labels for a given resource.
// additionalResourceLabels is optional.
// When omitted, only the default set of resource labels is exposed.
// Minimum length for this list is 1.
// Maximum length for this list is 2.
// Each resource name must be unique within this list.
// +optional
// +kubebuilder:validation:MaxItems=2
// +kubebuilder:validation:MinItems=1
// +listType=map
// +listMapKey=resource
AdditionalResourceLabels []KubeStateMetricsResourceLabels `json:"additionalResourceLabels,omitempty"`
}

// KubeStateMetricsResourceName is the name of a Kubernetes resource whose labels can be exposed
// as metrics by kube-state-metrics. Currently, only "Jobs" and "CronJobs" are supported
// due to cardinality concerns.
// Valid values are "Jobs" and "CronJobs".
// +kubebuilder:validation:Enum=Jobs;CronJobs
type KubeStateMetricsResourceName string
Comment on lines +2606 to +2607
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The enum values Jobs and CronJobs are neither the Kubernetes Kind (singular PascalCase: Job, CronJob) nor the resource name (lowercase plural: jobs, cronjobs). Per Kubernetes API conventions, enum values should be PascalCase, which matches the Kind form.

Should these be Job and CronJob instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback! This was actually addressed in an earlier review round with @simonpasquier, who suggested using Jobs and CronJobs to align with Kubernetes resource name conventions.

The CMO ConfigMap API uses lowercase plural (jobs/cronjobs), so IMHO plural is the natural CRD equivalent referencing the resource type rather than the Kind. Happy to discuss further if you feel strongly about singular form.


const (
// KubeStateMetricsResourceJobs indicates the Kubernetes Jobs resource.
KubeStateMetricsResourceJobs KubeStateMetricsResourceName = "Jobs"
// KubeStateMetricsResourceCronJobs indicates the Kubernetes CronJobs resource.
KubeStateMetricsResourceCronJobs KubeStateMetricsResourceName = "CronJobs"
)

// KubeStateMetricsLabelName is the name of a Kubernetes label to expose as a metric
// via kube-state-metrics. Use "*" to expose all labels for a resource.
// Must be either the wildcard "*" or a valid Kubernetes label key.
// A valid label key has an optional DNS subdomain prefix followed by a "/" and a name segment,
// or just a name segment without a prefix. The name segment must be 63 characters or fewer,
// beginning and ending with an alphanumeric character, with dashes, underscores, dots, and
// alphanumerics in between.
// Must be at least 1 character and at most 253 characters in length.
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +kubebuilder:validation:XValidation:rule="self == '*' || !format.qualifiedName().validate(self).hasValue()",message="must be a valid Kubernetes label key or the wildcard '*'"
type KubeStateMetricsLabelName string
Comment on lines +2616 to +2627
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The regex ^[a-zA-Z0-9][a-zA-Z0-9_./-]*[a-zA-Z0-9]$ is too permissive for Kubernetes label keys:

  • It allows multiple slashes (e.g., foo/bar/baz), but a valid label key has at most one slash separating prefix from name.
  • It allows uppercase in the DNS subdomain prefix, which is invalid.
  • It doesn't enforce the prefix/name structure at all.

The existing ContainerResource.Name field in this same file uses format.qualifiedName() for validation. Consider using the same approach here for consistency and correctness:

Suggested change
// KubeStateMetricsLabelName is the name of a Kubernetes label to expose as a metric
// via kube-state-metrics. Use "*" to expose all labels for a resource.
// Must be either the wildcard "*" or a valid Kubernetes label key.
// A valid label key has an optional DNS subdomain prefix followed by a "/" and a name segment,
// or just a name segment without a prefix. The name segment must be 63 characters or fewer,
// beginning and ending with an alphanumeric character, with dashes, underscores, dots, and
// alphanumerics in between.
// Must be at least 1 character and at most 253 characters in length.
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +kubebuilder:validation:XValidation:rule="self == '*' || self.matches('^[a-zA-Z0-9][a-zA-Z0-9_./-]*[a-zA-Z0-9]$') || self.matches('^[a-zA-Z0-9]$')",message="must be a valid Kubernetes label key or the wildcard '*'"
type KubeStateMetricsLabelName string
// KubeStateMetricsLabelName is the name of a Kubernetes label to expose as a metric
// via kube-state-metrics. Use "*" to expose all labels for a resource.
// Must be either the wildcard "*" or a valid Kubernetes label key.
// A valid label key has an optional DNS subdomain prefix followed by a "/" and a name segment,
// or just a name segment without a prefix. The name segment must be 63 characters or fewer,
// beginning and ending with an alphanumeric character, with dashes, underscores, dots, and
// alphanumerics in between.
// Must be at least 1 character and at most 253 characters in length.
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +kubebuilder:validation:XValidation:rule="self == '*' || !format.qualifiedName().validate(self).hasValue()",message="must be a valid Kubernetes label key or the wildcard '*'"
type KubeStateMetricsLabelName string

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed, the regex was too loose. Switched to format.qualifiedName() as suggested. Thanks!


// KubeStateMetricsResourceLabels defines which Kubernetes labels to expose as metrics
// for a given resource type in kube-state-metrics.
type KubeStateMetricsResourceLabels struct {
// resource is the Kubernetes resource name whose labels should be exposed as metrics.
// Currently, only "Jobs" and "CronJobs" are supported due to cardinality concerns.
// Valid values are "Jobs" and "CronJobs".
// This field is required.
// +required
Resource KubeStateMetricsResourceName `json:"resource,omitempty"`
// labels is the list of Kubernetes label names to expose as metrics for this resource.
// Use "*" to expose all labels for the specified resource.
// This field is required.
// Each label name must be unique within this list.
// Minimum length for this list is 1.
// Maximum length for this list is 50.
// +required
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=50
// +listType=set
Labels []KubeStateMetricsLabelName `json:"labels,omitempty"`
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Loading