From 5de2c504cb69c1055d580ba29f06f298ea0a2af7 Mon Sep 17 00:00:00 2001 From: Adam Bernot Date: Fri, 17 Jan 2025 19:07:56 +0000 Subject: [PATCH] refactor: default podmonitoring with CRDs --- ....googleapis.com_clusterpodmonitorings.yaml | 7 + ...itoring.googleapis.com_podmonitorings.yaml | 6 + .../mutatingwebhookconfiguration.yaml | 44 ----- manifests/operator.yaml | 44 ----- manifests/setup.yaml | 13 ++ pkg/operator/apis/monitoring/v1/pod_types.go | 32 +--- pkg/operator/collection.go | 10 +- pkg/operator/collection_test.go | 156 ------------------ pkg/operator/webhook.go | 8 - 9 files changed, 30 insertions(+), 290 deletions(-) diff --git a/charts/operator/crds/monitoring.googleapis.com_clusterpodmonitorings.yaml b/charts/operator/crds/monitoring.googleapis.com_clusterpodmonitorings.yaml index e2e9ca0d8d..34802f7938 100644 --- a/charts/operator/crds/monitoring.googleapis.com_clusterpodmonitorings.yaml +++ b/charts/operator/crds/monitoring.googleapis.com_clusterpodmonitorings.yaml @@ -568,6 +568,13 @@ spec: type: object x-kubernetes-map-type: atomic targetLabels: + default: + metadata: + - namespace + - pod + - container + - top_level_controller_name + - top_level_controller_type description: |- Labels to add to the Prometheus target for discovered endpoints. The `instance` label is always set to `:` or `:` diff --git a/charts/operator/crds/monitoring.googleapis.com_podmonitorings.yaml b/charts/operator/crds/monitoring.googleapis.com_podmonitorings.yaml index 24f6141b69..7b1364b4a3 100644 --- a/charts/operator/crds/monitoring.googleapis.com_podmonitorings.yaml +++ b/charts/operator/crds/monitoring.googleapis.com_podmonitorings.yaml @@ -563,6 +563,12 @@ spec: type: object x-kubernetes-map-type: atomic targetLabels: + default: + metadata: + - pod + - container + - top_level_controller_name + - top_level_controller_type description: |- Labels to add to the Prometheus target for discovered endpoints. The `instance` label is always set to `:` or `:` diff --git a/charts/operator/templates/mutatingwebhookconfiguration.yaml b/charts/operator/templates/mutatingwebhookconfiguration.yaml index 98c8645aa9..5be9c0b59d 100644 --- a/charts/operator/templates/mutatingwebhookconfiguration.yaml +++ b/charts/operator/templates/mutatingwebhookconfiguration.yaml @@ -22,50 +22,6 @@ metadata: {{- include "prometheus-engine.labels" . | nindent 4 }} {{- end }} webhooks: -- name: default.podmonitorings.gmp-operator.gmp-system.monitoring.googleapis.com - admissionReviewVersions: - - v1 - clientConfig: - # caBundle populated by operator. - service: - name: gmp-operator - namespace: {{.Values.namespace.system}} - port: 443 - path: /default/monitoring.googleapis.com/v1/podmonitorings - failurePolicy: Fail - rules: - - resources: - - podmonitorings - apiGroups: - - monitoring.googleapis.com - apiVersions: - - v1 - operations: - - CREATE - - UPDATE - sideEffects: None -- name: default.clusterpodmonitorings.gmp-operator.gmp-system.monitoring.googleapis.com - admissionReviewVersions: - - v1 - clientConfig: - # caBundle populated by operator. - service: - name: gmp-operator - namespace: {{.Values.namespace.system}} - port: 443 - path: /default/monitoring.googleapis.com/v1/clusterpodmonitorings - failurePolicy: Fail - rules: - - resources: - - clusterpodmonitorings - apiGroups: - - monitoring.googleapis.com - apiVersions: - - v1 - operations: - - CREATE - - UPDATE - sideEffects: None - name: default.operatorconfigs.gmp-operator.gmp-system.monitoring.googleapis.com admissionReviewVersions: - v1 diff --git a/manifests/operator.yaml b/manifests/operator.yaml index 8302df1b0e..2b242db57d 100644 --- a/manifests/operator.yaml +++ b/manifests/operator.yaml @@ -964,50 +964,6 @@ kind: MutatingWebhookConfiguration metadata: name: gmp-operator.gmp-system.monitoring.googleapis.com webhooks: -- name: default.podmonitorings.gmp-operator.gmp-system.monitoring.googleapis.com - admissionReviewVersions: - - v1 - clientConfig: - # caBundle populated by operator. - service: - name: gmp-operator - namespace: gmp-system - port: 443 - path: /default/monitoring.googleapis.com/v1/podmonitorings - failurePolicy: Fail - rules: - - resources: - - podmonitorings - apiGroups: - - monitoring.googleapis.com - apiVersions: - - v1 - operations: - - CREATE - - UPDATE - sideEffects: None -- name: default.clusterpodmonitorings.gmp-operator.gmp-system.monitoring.googleapis.com - admissionReviewVersions: - - v1 - clientConfig: - # caBundle populated by operator. - service: - name: gmp-operator - namespace: gmp-system - port: 443 - path: /default/monitoring.googleapis.com/v1/clusterpodmonitorings - failurePolicy: Fail - rules: - - resources: - - clusterpodmonitorings - apiGroups: - - monitoring.googleapis.com - apiVersions: - - v1 - operations: - - CREATE - - UPDATE - sideEffects: None - name: default.operatorconfigs.gmp-operator.gmp-system.monitoring.googleapis.com admissionReviewVersions: - v1 diff --git a/manifests/setup.yaml b/manifests/setup.yaml index 996b2198e8..eb286b30ae 100644 --- a/manifests/setup.yaml +++ b/manifests/setup.yaml @@ -774,6 +774,13 @@ spec: type: object x-kubernetes-map-type: atomic targetLabels: + default: + metadata: + - namespace + - pod + - container + - top_level_controller_name + - top_level_controller_type description: |- Labels to add to the Prometheus target for discovered endpoints. The `instance` label is always set to `:` or `:` @@ -2952,6 +2959,12 @@ spec: type: object x-kubernetes-map-type: atomic targetLabels: + default: + metadata: + - pod + - container + - top_level_controller_name + - top_level_controller_type description: |- Labels to add to the Prometheus target for discovered endpoints. The `instance` label is always set to `:` or `:` diff --git a/pkg/operator/apis/monitoring/v1/pod_types.go b/pkg/operator/apis/monitoring/v1/pod_types.go index a30e454d5a..b9e6e2ee5a 100644 --- a/pkg/operator/apis/monitoring/v1/pod_types.go +++ b/pkg/operator/apis/monitoring/v1/pod_types.go @@ -106,21 +106,6 @@ func (p *PodMonitoring) ValidateDelete() (admission.Warnings, error) { return nil, nil } -// Default implements admission.Defaulter. -func (p *PodMonitoring) Default() { - _ = p.UpdateDefault() -} - -// UpdateDefault defaults any unset fields, returning true if the object was updated. -func (p *PodMonitoring) UpdateDefault() bool { - if p.Spec.TargetLabels.Metadata == nil { - md := []string{"pod", "container", "top_level_controller_name", "top_level_controller_type"} - p.Spec.TargetLabels.Metadata = &md - return true - } - return false -} - // PodMonitoringList is a list of PodMonitorings. // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object type PodMonitoringList struct { @@ -186,21 +171,6 @@ func (*ClusterPodMonitoring) ValidateDelete() (admission.Warnings, error) { return nil, nil } -// Default implements admission.Defaulter. -func (c *ClusterPodMonitoring) Default() { - _ = c.UpdateDefault() -} - -// UpdateDefault defaults any unset fields, returning true if the object was updated. -func (c *ClusterPodMonitoring) UpdateDefault() bool { - if c.Spec.TargetLabels.Metadata == nil { - md := []string{"namespace", "pod", "container", "top_level_controller_name", "top_level_controller_type"} - c.Spec.TargetLabels.Metadata = &md - return true - } - return false -} - // ClusterPodMonitoringList is a list of ClusterPodMonitorings. // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object type ClusterPodMonitoringList struct { @@ -219,6 +189,7 @@ type PodMonitoringSpec struct { // Labels to add to the Prometheus target for discovered endpoints. // The `instance` label is always set to `:` or `:` // if the scraped pod is controlled by a DaemonSet. + //+kubebuilder:default={metadata:{pod,container,top_level_controller_name,top_level_controller_type}} TargetLabels TargetLabels `json:"targetLabels,omitempty"` // Limits to apply at scrape time. Limits *ScrapeLimits `json:"limits,omitempty"` @@ -254,6 +225,7 @@ type ClusterPodMonitoringSpec struct { // Labels to add to the Prometheus target for discovered endpoints. // The `instance` label is always set to `:` or `:` // if the scraped pod is controlled by a DaemonSet. + //+kubebuilder:default={metadata:{namespace,pod,container,top_level_controller_name,top_level_controller_type}} TargetLabels TargetLabels `json:"targetLabels,omitempty"` // Limits to apply at scrape time. Limits *ScrapeLimits `json:"limits,omitempty"` diff --git a/pkg/operator/collection.go b/pkg/operator/collection.go index 3b9ae1e294..deeea09d99 100644 --- a/pkg/operator/collection.go +++ b/pkg/operator/collection.go @@ -400,8 +400,6 @@ func (r *collectionReconciler) makeCollectorConfig(ctx context.Context, spec *mo // Mark status updates in batch with single timestamp. for _, pmon := range podMons.Items { - updateSpec := pmon.UpdateDefault() - cond := &monitoringv1.MonitoringCondition{ Type: monitoringv1.ConfigurationCreateSuccess, Status: corev1.ConditionTrue, @@ -421,10 +419,9 @@ func (r *collectionReconciler) makeCollectorConfig(ctx context.Context, spec *mo } updateStatus := pmon.Status.SetMonitoringCondition(pmon.GetGeneration(), metav1.Now(), cond) - if updateSpec || updateStatus { + if updateStatus { updates = append(updates, update{ object: &pmon, - spec: updateSpec, status: updateStatus, }) } @@ -436,8 +433,6 @@ func (r *collectionReconciler) makeCollectorConfig(ctx context.Context, spec *mo // Mark status updates in batch with single timestamp. for _, cmon := range clusterPodMons.Items { - updateSpec := cmon.UpdateDefault() - cond := &monitoringv1.MonitoringCondition{ Type: monitoringv1.ConfigurationCreateSuccess, Status: corev1.ConditionTrue, @@ -457,10 +452,9 @@ func (r *collectionReconciler) makeCollectorConfig(ctx context.Context, spec *mo } updateStatus := cmon.Status.SetMonitoringCondition(cmon.GetGeneration(), metav1.Now(), cond) - if updateSpec || updateStatus { + if updateStatus { updates = append(updates, update{ object: &cmon, - spec: updateSpec, status: updateStatus, }) } diff --git a/pkg/operator/collection_test.go b/pkg/operator/collection_test.go index 9c87d73af7..8d6aa70363 100644 --- a/pkg/operator/collection_test.go +++ b/pkg/operator/collection_test.go @@ -220,84 +220,6 @@ func TestCollectionReconcile(t *testing.T) { }, }, }, - { - desc: "podmonitoring: default spec", - input: &monitoringv1.PodMonitoring{ - ObjectMeta: exampleObjectMeta, - Spec: monitoringv1.PodMonitoringSpec{ - Endpoints: validScrapeEndpoints, - }, - Status: monitoringv1.PodMonitoringStatus{ - EndpointStatuses: exampleEndpointStatus, - MonitoringStatus: monitoringv1.MonitoringStatus{ - Conditions: []monitoringv1.MonitoringCondition{ - { - Type: "ConfigurationCreateSuccess", - Status: corev1.ConditionTrue, - }, - }, - }, - }, - }, - expected: &monitoringv1.PodMonitoring{ - ObjectMeta: metav1.ObjectMeta{ - Name: "prom-example", - Namespace: "gmp-test", - ResourceVersion: "2", - }, Spec: monitoringv1.PodMonitoringSpec{ - TargetLabels: monitoringv1.TargetLabels{ - Metadata: &[]string{"pod", "container", "top_level_controller_name", "top_level_controller_type"}, - }, - Endpoints: validScrapeEndpoints, - }, - Status: monitoringv1.PodMonitoringStatus{ - EndpointStatuses: exampleEndpointStatus, - MonitoringStatus: monitoringv1.MonitoringStatus{ - Conditions: []monitoringv1.MonitoringCondition{ - { - Type: "ConfigurationCreateSuccess", - Status: corev1.ConditionTrue, - }, - }, - }, - }, - }, - }, - { - desc: "podmonitoring: default spec and update status", - input: &monitoringv1.PodMonitoring{ - ObjectMeta: exampleObjectMeta, - Spec: monitoringv1.PodMonitoringSpec{ - Endpoints: validScrapeEndpoints, - }, - Status: monitoringv1.PodMonitoringStatus{ - EndpointStatuses: exampleEndpointStatus, - }, - }, - expected: &monitoringv1.PodMonitoring{ - ObjectMeta: metav1.ObjectMeta{ - Name: "prom-example", - Namespace: "gmp-test", - ResourceVersion: "3", - }, Spec: monitoringv1.PodMonitoringSpec{ - TargetLabels: monitoringv1.TargetLabels{ - Metadata: &[]string{"pod", "container", "top_level_controller_name", "top_level_controller_type"}, - }, - Endpoints: validScrapeEndpoints, - }, - Status: monitoringv1.PodMonitoringStatus{ - EndpointStatuses: exampleEndpointStatus, - MonitoringStatus: monitoringv1.MonitoringStatus{ - Conditions: []monitoringv1.MonitoringCondition{ - { - Type: "ConfigurationCreateSuccess", - Status: corev1.ConditionTrue, - }, - }, - }, - }, - }, - }, { desc: "clusterpodmonitoring: no update", input: &monitoringv1.ClusterPodMonitoring{ @@ -404,84 +326,6 @@ func TestCollectionReconcile(t *testing.T) { }, }, }, - { - desc: "clusterpodmonitoring: default spec", - input: &monitoringv1.ClusterPodMonitoring{ - ObjectMeta: exampleObjectMeta, - Spec: monitoringv1.ClusterPodMonitoringSpec{ - Endpoints: validScrapeEndpoints, - }, - Status: monitoringv1.PodMonitoringStatus{ - EndpointStatuses: exampleEndpointStatus, - MonitoringStatus: monitoringv1.MonitoringStatus{ - Conditions: []monitoringv1.MonitoringCondition{ - { - Type: "ConfigurationCreateSuccess", - Status: corev1.ConditionTrue, - }, - }, - }, - }, - }, - expected: &monitoringv1.ClusterPodMonitoring{ - ObjectMeta: metav1.ObjectMeta{ - Name: "prom-example", - Namespace: "gmp-test", - ResourceVersion: "2", - }, Spec: monitoringv1.ClusterPodMonitoringSpec{ - TargetLabels: monitoringv1.TargetLabels{ - Metadata: &[]string{"namespace", "pod", "container", "top_level_controller_name", "top_level_controller_type"}, - }, - Endpoints: validScrapeEndpoints, - }, - Status: monitoringv1.PodMonitoringStatus{ - EndpointStatuses: exampleEndpointStatus, - MonitoringStatus: monitoringv1.MonitoringStatus{ - Conditions: []monitoringv1.MonitoringCondition{ - { - Type: "ConfigurationCreateSuccess", - Status: corev1.ConditionTrue, - }, - }, - }, - }, - }, - }, - { - desc: "clusterpodmonitoring: default spec and update status", - input: &monitoringv1.ClusterPodMonitoring{ - ObjectMeta: exampleObjectMeta, - Spec: monitoringv1.ClusterPodMonitoringSpec{ - Endpoints: validScrapeEndpoints, - }, - Status: monitoringv1.PodMonitoringStatus{ - EndpointStatuses: exampleEndpointStatus, - }, - }, - expected: &monitoringv1.ClusterPodMonitoring{ - ObjectMeta: metav1.ObjectMeta{ - Name: "prom-example", - Namespace: "gmp-test", - ResourceVersion: "3", - }, Spec: monitoringv1.ClusterPodMonitoringSpec{ - TargetLabels: monitoringv1.TargetLabels{ - Metadata: &[]string{"namespace", "pod", "container", "top_level_controller_name", "top_level_controller_type"}, - }, - Endpoints: validScrapeEndpoints, - }, - Status: monitoringv1.PodMonitoringStatus{ - EndpointStatuses: exampleEndpointStatus, - MonitoringStatus: monitoringv1.MonitoringStatus{ - Conditions: []monitoringv1.MonitoringCondition{ - { - Type: "ConfigurationCreateSuccess", - Status: corev1.ConditionTrue, - }, - }, - }, - }, - }, - }, { desc: "clusternodemonitoring: no update", input: &monitoringv1.ClusterNodeMonitoring{ diff --git a/pkg/operator/webhook.go b/pkg/operator/webhook.go index 5d352452ee..2d3947b2c7 100644 --- a/pkg/operator/webhook.go +++ b/pkg/operator/webhook.go @@ -89,14 +89,6 @@ func setupAdmissionWebhooks(ctx context.Context, logger logr.Logger, kubeClient admission.ValidatingWebhookFor(scheme, &monitoringv1.GlobalRules{}), ) // Defaulting webhooks. - webhookServer.Register( - defaultPath(monitoringv1.PodMonitoringResource()), - admission.DefaultingWebhookFor(scheme, &monitoringv1.PodMonitoring{}), - ) - webhookServer.Register( - defaultPath(monitoringv1.ClusterPodMonitoringResource()), - admission.DefaultingWebhookFor(scheme, &monitoringv1.ClusterPodMonitoring{}), - ) webhookServer.Register( defaultPath(monitoringv1.OperatorConfigResource()), admission.WithCustomDefaulter(scheme, &monitoringv1.OperatorConfig{}, &operatorConfigDefaulter{