Skip to content

Commit 484f320

Browse files
rexagodrexagod
committed
OCPBUGS-17113: Lazy updates for Prometheus
Incorporate openshift/library-go#1575 downstream. Signed-off-by: Pranshu Srivastava <[email protected]>
1 parent 5d1fd1b commit 484f320

File tree

10 files changed

+600
-159
lines changed

10 files changed

+600
-159
lines changed

‎go.mod

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ require (
1212
github.com/ghodss/yaml v1.0.0
1313
github.com/go-kit/log v0.2.1
1414
github.com/go-openapi/strfmt v0.22.0
15+
github.com/google/go-cmp v0.6.0
1516
github.com/google/uuid v1.6.0
1617
github.com/imdario/mergo v0.3.16
1718
github.com/openshift/api v0.0.0-20240618205917-987b8890c273
@@ -81,7 +82,6 @@ require (
8182
github.com/golang/protobuf v1.5.4 // indirect
8283
github.com/google/cel-go v0.17.8 // indirect
8384
github.com/google/gnostic-models v0.6.8 // indirect
84-
github.com/google/go-cmp v0.6.0 // indirect
8585
github.com/google/gofuzz v1.2.0 // indirect
8686
github.com/grafana/regexp v0.0.0-20221122212121-6b5c0a4cb7fd // indirect
8787
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect
@@ -148,3 +148,5 @@ require (
148148
sigs.k8s.io/kube-storage-version-migrator v0.0.6-0.20230721195810-5c8923c5ff96 // indirect
149149
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
150150
)
151+
152+
replace github.com/openshift/library-go => github.com/rexagod/library-go v0.0.0-20240625152537-56a2f7ff3336

‎go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,8 +396,6 @@ github.com/openshift/api v0.0.0-20240618205917-987b8890c273 h1:a2B5ocKga0ckZlb4f
396396
github.com/openshift/api v0.0.0-20240618205917-987b8890c273/go.mod h1:OOh6Qopf21pSzqNVCB5gomomBXb8o5sGKZxG2KNpaXM=
397397
github.com/openshift/client-go v0.0.0-20240528061634-b054aa794d87 h1:JtLhaGpSEconE+1IKmIgCOof/Len5ceG6H1pk43yv5U=
398398
github.com/openshift/client-go v0.0.0-20240528061634-b054aa794d87/go.mod h1:3IPD4U0qyovZS4EFady2kqY32m8lGcbs/Wx+yprg9z8=
399-
github.com/openshift/library-go v0.0.0-20240619120114-0c65da30ad30 h1:c9PNqAVBbnsR4Ro+P2e2Ih3aacnq5l1IfGX5985Rd7c=
400-
github.com/openshift/library-go v0.0.0-20240619120114-0c65da30ad30/go.mod h1:PdASVamWinll2BPxiUpXajTwZxV8A1pQbWEsCN1od7I=
401399
github.com/ovh/go-ovh v1.4.3 h1:Gs3V823zwTFpzgGLZNI6ILS4rmxZgJwJCz54Er9LwD0=
402400
github.com/ovh/go-ovh v1.4.3/go.mod h1:AkPXVtgwB6xlKblMjRKJJmjRp+ogrE7fz2lVgcQY8SY=
403401
github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU=
@@ -443,6 +441,8 @@ github.com/prometheus/procfs v0.12.0 h1:jluTpSng7V9hY0O2R9DzzJHYb2xULk9VTR1V1R/k
443441
github.com/prometheus/procfs v0.12.0/go.mod h1:pcuDEFsWDnvcgNzo4EEweacyhjeA9Zk3cnaOZAZEfOo=
444442
github.com/prometheus/prometheus v0.48.1 h1:CTszphSNTXkuCG6O0IfpKdHcJkvvnAAE1GbELKS+NFk=
445443
github.com/prometheus/prometheus v0.48.1/go.mod h1:SRw624aMAxTfryAcP8rOjg4S/sHHaetx2lyJJ2nM83g=
444+
github.com/rexagod/library-go v0.0.0-20240625152537-56a2f7ff3336 h1:weB3nceKGmmC3JREE/OelCSILuUATFHDgjhVVjcYgoI=
445+
github.com/rexagod/library-go v0.0.0-20240625152537-56a2f7ff3336/go.mod h1:/wmao3qtqOQ484HDka9cWP7SIvOQOdzpmhyXkF2YdzE=
446446
github.com/robfig/cron v1.2.0 h1:ZjScXvvxeQ63Dbyxy76Fj3AT3Ut0aKsyd2/tl3DTMuQ=
447447
github.com/robfig/cron v1.2.0/go.mod h1:JGuDeoQd7Z6yL4zQhZ3OPEVHB7fL6Ka6skscFHfmt2k=
448448
github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=

‎pkg/client/client.go

Lines changed: 128 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,6 @@ import (
2525
"strings"
2626
"time"
2727

28-
"github.com/imdario/mergo"
29-
configv1 "github.com/openshift/api/config/v1"
30-
consolev1 "github.com/openshift/api/console/v1"
31-
osmv1 "github.com/openshift/api/monitoring/v1"
32-
routev1 "github.com/openshift/api/route/v1"
33-
secv1 "github.com/openshift/api/security/v1"
34-
openshiftconfigclientset "github.com/openshift/client-go/config/clientset/versioned"
35-
openshiftconsoleclientset "github.com/openshift/client-go/console/clientset/versioned"
36-
openshiftmonitoringclientset "github.com/openshift/client-go/monitoring/clientset/versioned"
37-
openshiftoperatorclientset "github.com/openshift/client-go/operator/clientset/versioned"
38-
openshiftrouteclientset "github.com/openshift/client-go/route/clientset/versioned"
39-
openshiftsecurityclientset "github.com/openshift/client-go/security/clientset/versioned"
40-
"github.com/openshift/library-go/pkg/operator/events"
41-
"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
42-
monv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
43-
monitoring "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned"
4428
admissionv1 "k8s.io/api/admissionregistration/v1"
4529
appsv1 "k8s.io/api/apps/v1"
4630
v1 "k8s.io/api/core/v1"
@@ -50,6 +34,7 @@ import (
5034
apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
5135
apierrors "k8s.io/apimachinery/pkg/api/errors"
5236
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
37+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
5338
"k8s.io/apimachinery/pkg/fields"
5439
"k8s.io/apimachinery/pkg/runtime"
5540
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -58,6 +43,7 @@ import (
5843
"k8s.io/apimachinery/pkg/util/intstr"
5944
"k8s.io/apimachinery/pkg/util/wait"
6045
"k8s.io/apimachinery/pkg/watch"
46+
"k8s.io/client-go/dynamic"
6147
"k8s.io/client-go/kubernetes"
6248
"k8s.io/client-go/metadata"
6349
"k8s.io/client-go/rest"
@@ -66,6 +52,23 @@ import (
6652
apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
6753
aggregatorclient "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset"
6854
"k8s.io/utils/ptr"
55+
56+
"github.com/imdario/mergo"
57+
configv1 "github.com/openshift/api/config/v1"
58+
consolev1 "github.com/openshift/api/console/v1"
59+
osmv1 "github.com/openshift/api/monitoring/v1"
60+
routev1 "github.com/openshift/api/route/v1"
61+
secv1 "github.com/openshift/api/security/v1"
62+
openshiftconfigclientset "github.com/openshift/client-go/config/clientset/versioned"
63+
openshiftconsoleclientset "github.com/openshift/client-go/console/clientset/versioned"
64+
openshiftmonitoringclientset "github.com/openshift/client-go/monitoring/clientset/versioned"
65+
openshiftoperatorclientset "github.com/openshift/client-go/operator/clientset/versioned"
66+
openshiftrouteclientset "github.com/openshift/client-go/route/clientset/versioned"
67+
openshiftsecurityclientset "github.com/openshift/client-go/security/clientset/versioned"
68+
"github.com/openshift/library-go/pkg/operator/events"
69+
"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
70+
monv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
71+
monitoring "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned"
6972
)
7073

7174
const (
@@ -81,6 +84,7 @@ type Client struct {
8184
userWorkloadNamespace string
8285

8386
kclient kubernetes.Interface
87+
dclient dynamic.Interface
8488
mdataclient metadata.Interface
8589
osmclient openshiftmonitoringclientset.Interface
8690
oscclient openshiftconfigclientset.Interface
@@ -107,6 +111,14 @@ func NewForConfig(cfg *rest.Config, version string, namespace, userWorkloadNames
107111
client.kclient = kclient
108112
}
109113

114+
if client.dclient == nil {
115+
dclient, err := dynamic.NewForConfig(cfg)
116+
if err != nil {
117+
return nil, fmt.Errorf("creating dynamic clientset client: %w", err)
118+
}
119+
client.dclient = dclient
120+
}
121+
110122
if client.eclient == nil {
111123
eclient, err := apiextensionsclient.NewForConfig(cfg)
112124
if err != nil {
@@ -203,6 +215,12 @@ func KubernetesClient(kclient kubernetes.Interface) Option {
203215
}
204216
}
205217

218+
func DynamicClient(dclient *dynamic.DynamicClient) Option {
219+
return func(c *Client) {
220+
c.dclient = dclient
221+
}
222+
}
223+
206224
func OpenshiftMonitoringClient(osmclient openshiftmonitoringclientset.Interface) Option {
207225
return func(c *Client) {
208226
c.osmclient = osmclient
@@ -632,29 +650,96 @@ func (c *Client) GetAlertingRule(ctx context.Context, namespace, name string) (*
632650
return c.osmclient.MonitoringV1().AlertingRules(namespace).Get(ctx, name, metav1.GetOptions{})
633651
}
634652

635-
func (c *Client) CreateOrUpdatePrometheus(ctx context.Context, p *monv1.Prometheus) error {
636-
pclient := c.mclient.MonitoringV1().Prometheuses(p.GetNamespace())
637-
existing, err := pclient.Get(ctx, p.GetName(), metav1.GetOptions{})
653+
func (c *Client) CreateOrUpdatePrometheus(ctx context.Context, structuredRequiredPrometheus *monv1.Prometheus) (*bool, error) {
654+
unstructuredRequiredPrometheusObject, err := runtime.DefaultUnstructuredConverter.ToUnstructured(structuredRequiredPrometheus)
655+
if err != nil {
656+
return nil, fmt.Errorf("converting Prometheus object to unstructured failed: %w", err)
657+
}
658+
unstructuredRequiredPrometheus := &unstructured.Unstructured{}
659+
unstructuredRequiredPrometheus.SetUnstructuredContent(unstructuredRequiredPrometheusObject)
660+
661+
// Preserve the original behavior: always merge the metadata, never replace.
662+
// Refer: https://github.com/openshift/cluster-monitoring-operator/pull/942.
663+
unstructuredExistingPrometheus, err := c.dclient.Resource(structuredRequiredPrometheus.GroupVersionKind().GroupVersion().WithResource("prometheuses")).Namespace(structuredRequiredPrometheus.GetNamespace()).Get(ctx, structuredRequiredPrometheus.GetName(), metav1.GetOptions{})
638664
if apierrors.IsNotFound(err) {
639-
_, err := pclient.Create(ctx, p, metav1.CreateOptions{})
665+
_, err := c.dclient.Resource(structuredRequiredPrometheus.GroupVersionKind().GroupVersion().WithResource("prometheuses")).Namespace(structuredRequiredPrometheus.GetNamespace()).Create(ctx, unstructuredRequiredPrometheus, metav1.CreateOptions{})
640666
if err != nil {
641-
return fmt.Errorf("creating Prometheus object failed: %w", err)
667+
return nil, fmt.Errorf("creating Prometheus object failed: %w", err)
642668
}
643-
return nil
669+
return ptr.To(true), nil
644670
}
645671
if err != nil {
646-
return fmt.Errorf("retrieving Prometheus object failed: %w", err)
672+
return nil, fmt.Errorf("retrieving Prometheus object failed: %w", err)
647673
}
674+
unstructuredRequiredPrometheusMetadataLabels := unstructuredRequiredPrometheus.GetLabels()
675+
unstructuredRequiredPrometheusMetadataAnnotations := unstructuredRequiredPrometheus.GetAnnotations()
676+
mergeMetadataLabels(unstructuredRequiredPrometheusMetadataLabels, unstructuredExistingPrometheus.GetLabels())
677+
mergeMetadataAnnotations(unstructuredRequiredPrometheusMetadataAnnotations, unstructuredExistingPrometheus.GetAnnotations())
678+
unstructuredRequiredPrometheus.SetLabels(unstructuredRequiredPrometheusMetadataLabels)
679+
unstructuredRequiredPrometheus.SetAnnotations(unstructuredRequiredPrometheusMetadataAnnotations)
680+
unstructuredRequiredPrometheus.SetResourceVersion(unstructuredExistingPrometheus.GetResourceVersion())
648681

649-
required := p.DeepCopy()
650-
mergeMetadata(&required.ObjectMeta, existing.ObjectMeta)
682+
_, didUpdate, err := resourceapply.ApplyUnstructuredResourceImproved(
683+
ctx,
684+
c.dclient,
685+
c.eventRecorder,
686+
unstructuredRequiredPrometheus,
687+
c.resourceCache,
688+
structuredRequiredPrometheus.GroupVersionKind().GroupVersion().WithResource("prometheuses"),
689+
prometheusDefaultingFunc,
690+
nil,
691+
)
692+
if err != nil {
693+
return &didUpdate, fmt.Errorf("updating Prometheus object failed: %w", err)
694+
}
651695

652-
required.ResourceVersion = existing.ResourceVersion
653-
_, err = pclient.Update(ctx, required, metav1.UpdateOptions{})
696+
return &didUpdate, nil
697+
}
698+
699+
func prometheusDefaultingFunc(unstructuredPrometheus *unstructured.Unstructured) {
700+
// Cast to the corresponding structured representation.
701+
structuredPrometheus := &monv1.Prometheus{}
702+
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredPrometheus.UnstructuredContent(), structuredPrometheus); err != nil {
703+
klog.ErrorS(err, "failed to convert unstructured to structured Prometheus spec")
704+
return
705+
}
706+
707+
// Set defaults.
708+
if structuredPrometheus.Spec.CommonPrometheusFields.ScrapeInterval == "" {
709+
structuredPrometheus.Spec.CommonPrometheusFields.ScrapeInterval = "30s"
710+
}
711+
if len(structuredPrometheus.Spec.CommonPrometheusFields.ExternalLabels) == 0 {
712+
structuredPrometheus.Spec.CommonPrometheusFields.ExternalLabels = nil
713+
}
714+
if len(structuredPrometheus.Spec.CommonPrometheusFields.EnableFeatures) == 0 {
715+
structuredPrometheus.Spec.CommonPrometheusFields.EnableFeatures = nil
716+
}
717+
for i, container := range structuredPrometheus.Spec.CommonPrometheusFields.Containers {
718+
for j, port := range container.Ports {
719+
if port.Protocol == "" {
720+
structuredPrometheus.Spec.CommonPrometheusFields.Containers[i].Ports[j].Protocol = "TCP"
721+
}
722+
}
723+
}
724+
if structuredPrometheus.Spec.CommonPrometheusFields.PortName == "" {
725+
structuredPrometheus.Spec.CommonPrometheusFields.PortName = "web"
726+
}
727+
if structuredPrometheus.Spec.Thanos == nil {
728+
structuredPrometheus.Spec.Thanos = &monv1.ThanosSpec{}
729+
}
730+
if structuredPrometheus.Spec.Thanos.BlockDuration == "" {
731+
structuredPrometheus.Spec.Thanos.BlockDuration = "2h"
732+
}
733+
if structuredPrometheus.Spec.EvaluationInterval == "" {
734+
structuredPrometheus.Spec.EvaluationInterval = "30s"
735+
}
736+
737+
// Convert back to the corresponding unstructured representation and inject.
738+
var err error
739+
unstructuredPrometheus.Object, err = runtime.DefaultUnstructuredConverter.ToUnstructured(structuredPrometheus)
654740
if err != nil {
655-
return fmt.Errorf("updating Prometheus object failed: %w", err)
741+
klog.ErrorS(err, "failed to convert structured to unstructured Prometheus")
656742
}
657-
return nil
658743
}
659744

660745
func (c *Client) CreateOrUpdatePrometheusRule(ctx context.Context, p *monv1.PrometheusRule) error {
@@ -1788,20 +1873,28 @@ func (c *Client) VPACustomResourceDefinitionPresent(ctx context.Context, lastKno
17881873
// where keys starting from string defined in `metadataPrefix` are deleted. This prevents issues with preserving stale
17891874
// metadata defined by the operator
17901875
func mergeMetadata(required *metav1.ObjectMeta, existing metav1.ObjectMeta) {
1791-
for k := range existing.Annotations {
1876+
mergeMetadataLabels(required.Labels, existing.Labels)
1877+
mergeMetadataAnnotations(required.Annotations, existing.Annotations)
1878+
}
1879+
1880+
func mergeMetadataLabels(requiredLabels map[string]string, existingLabels map[string]string) {
1881+
for k := range existingLabels {
17921882
if strings.HasPrefix(k, metadataPrefix) {
1793-
delete(existing.Annotations, k)
1883+
delete(existingLabels, k)
17941884
}
17951885
}
17961886

1797-
for k := range existing.Labels {
1887+
_ = mergo.Merge(&requiredLabels, existingLabels)
1888+
}
1889+
1890+
func mergeMetadataAnnotations(requiredAnnotations map[string]string, existingAnnotations map[string]string) {
1891+
for k := range existingAnnotations {
17981892
if strings.HasPrefix(k, metadataPrefix) {
1799-
delete(existing.Labels, k)
1893+
delete(existingAnnotations, k)
18001894
}
18011895
}
18021896

1803-
_ = mergo.Merge(&required.Annotations, existing.Annotations)
1804-
_ = mergo.Merge(&required.Labels, existing.Labels)
1897+
_ = mergo.Merge(&requiredAnnotations, existingAnnotations)
18051898
}
18061899

18071900
type jsonPatch struct {

0 commit comments

Comments
 (0)