Skip to content

Commit

Permalink
OCPBUGS-17113: Lazy updates for Prometheus
Browse files Browse the repository at this point in the history
Incorporate openshift/library-go#1575
downstream.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
  • Loading branch information
rexagod committed Sep 9, 2024
1 parent c5668e3 commit 15b56ea
Show file tree
Hide file tree
Showing 30 changed files with 953 additions and 376 deletions.
10 changes: 7 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ GOPROXY?=http://proxy.golang.org
export GO111MODULE
export GOPROXY

# go pakages for unit tests, excluding e2e tests
# go packages for unit tests, excluding e2e tests
PKGS=$(shell go list ./... | grep -v /test/e2e)
GOLANG_FILES:=$(shell find . -name \*.go -print)
# NOTE: grep -v %.yaml is needed because "%s-policy.yaml" is used
Expand Down Expand Up @@ -249,17 +249,21 @@ check-runbooks:
# Testing #
###########

.PHONY: bench
bench:
go test -run NONE -bench ^Bench -benchmem $(PKGS)

.PHONY: test
test: test-unit test-rules test-e2e

.PHONY: test-unit
test-unit:
go test -race -short $(PKGS) -count=1
go test -run ^Test -race -short $(PKGS) -count=1

.PHONY: test-e2e
test-e2e: KUBECONFIG?=$(HOME)/.kube/config
test-e2e:
go test -v -timeout=150m ./test/e2e/ --kubeconfig $(KUBECONFIG)
go test -run ^Test -v -timeout=150m ./test/e2e/ --kubeconfig $(KUBECONFIG)

$(BIN_DIR):
mkdir -p $(BIN_DIR)
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@ require (
github.com/ghodss/yaml v1.0.0
github.com/go-kit/log v0.2.1
github.com/go-openapi/strfmt v0.22.0
github.com/google/go-cmp v0.6.0
github.com/google/uuid v1.6.0
github.com/imdario/mergo v0.3.16
github.com/openshift/api v0.0.0-20240618205917-987b8890c273
github.com/openshift/client-go v0.0.0-20240528061634-b054aa794d87
github.com/openshift/library-go v0.0.0-20240619120114-0c65da30ad30
github.com/openshift/library-go v0.0.0-20240829153658-aaabd80939d3
github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.76.0
github.com/prometheus-operator/prometheus-operator/pkg/client v0.76.0
github.com/prometheus/alertmanager v0.27.0
Expand Down Expand Up @@ -82,7 +83,6 @@ require (
github.com/golang/protobuf v1.5.4 // indirect
github.com/google/cel-go v0.17.8 // indirect
github.com/google/gnostic-models v0.6.8 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/grafana/regexp v0.0.0-20221122212121-6b5c0a4cb7fd // indirect
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,8 @@ github.com/openshift/api v0.0.0-20240618205917-987b8890c273 h1:a2B5ocKga0ckZlb4f
github.com/openshift/api v0.0.0-20240618205917-987b8890c273/go.mod h1:OOh6Qopf21pSzqNVCB5gomomBXb8o5sGKZxG2KNpaXM=
github.com/openshift/client-go v0.0.0-20240528061634-b054aa794d87 h1:JtLhaGpSEconE+1IKmIgCOof/Len5ceG6H1pk43yv5U=
github.com/openshift/client-go v0.0.0-20240528061634-b054aa794d87/go.mod h1:3IPD4U0qyovZS4EFady2kqY32m8lGcbs/Wx+yprg9z8=
github.com/openshift/library-go v0.0.0-20240619120114-0c65da30ad30 h1:c9PNqAVBbnsR4Ro+P2e2Ih3aacnq5l1IfGX5985Rd7c=
github.com/openshift/library-go v0.0.0-20240619120114-0c65da30ad30/go.mod h1:PdASVamWinll2BPxiUpXajTwZxV8A1pQbWEsCN1od7I=
github.com/openshift/library-go v0.0.0-20240829153658-aaabd80939d3 h1:te32Z4TU4AZ+Jp+Z/KavJCm9phgr9Hk5EDDLBJCnSgk=
github.com/openshift/library-go v0.0.0-20240829153658-aaabd80939d3/go.mod h1:/wmao3qtqOQ484HDka9cWP7SIvOQOdzpmhyXkF2YdzE=
github.com/ovh/go-ovh v1.4.3 h1:Gs3V823zwTFpzgGLZNI6ILS4rmxZgJwJCz54Er9LwD0=
github.com/ovh/go-ovh v1.4.3/go.mod h1:AkPXVtgwB6xlKblMjRKJJmjRp+ogrE7fz2lVgcQY8SY=
github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU=
Expand Down
106 changes: 86 additions & 20 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import (
apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand All @@ -58,6 +59,7 @@ import (
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/metadata"
"k8s.io/client-go/rest"
Expand All @@ -81,6 +83,7 @@ type Client struct {
userWorkloadNamespace string

kclient kubernetes.Interface
dclient dynamic.Interface
mdataclient metadata.Interface
osmclient openshiftmonitoringclientset.Interface
oscclient openshiftconfigclientset.Interface
Expand All @@ -107,6 +110,14 @@ func NewForConfig(cfg *rest.Config, version string, namespace, userWorkloadNames
client.kclient = kclient
}

if client.dclient == nil {
dclient, err := dynamic.NewForConfig(cfg)
if err != nil {
return nil, fmt.Errorf("creating dynamic clientset client: %w", err)
}
client.dclient = dclient
}

if client.eclient == nil {
eclient, err := apiextensionsclient.NewForConfig(cfg)
if err != nil {
Expand Down Expand Up @@ -203,6 +214,12 @@ func KubernetesClient(kclient kubernetes.Interface) Option {
}
}

func DynamicClient(dclient *dynamic.DynamicClient) Option {
return func(c *Client) {
c.dclient = dclient
}
}

func OpenshiftMonitoringClient(osmclient openshiftmonitoringclientset.Interface) Option {
return func(c *Client) {
c.osmclient = osmclient
Expand Down Expand Up @@ -632,29 +649,70 @@ func (c *Client) GetAlertingRule(ctx context.Context, namespace, name string) (*
return c.osmclient.MonitoringV1().AlertingRules(namespace).Get(ctx, name, metav1.GetOptions{})
}

func (c *Client) CreateOrUpdatePrometheus(ctx context.Context, p *monv1.Prometheus) error {
pclient := c.mclient.MonitoringV1().Prometheuses(p.GetNamespace())
existing, err := pclient.Get(ctx, p.GetName(), metav1.GetOptions{})
// NOTE: We don't use a defaulting function here, and resort only to the caching mechanism, since,
// * defaults for Prometheus should be introduced from its operator level, and,
// * the defaulting approach generally requires hardcoding the default values, which adds on to the maintenance overhead.
func (c *Client) CreateOrUpdatePrometheus(ctx context.Context, structuredRequiredPrometheus *monv1.Prometheus) (bool, error) {
unstructuredRequiredPrometheusObject, err := runtime.DefaultUnstructuredConverter.ToUnstructured(structuredRequiredPrometheus)
if err != nil {
return false, fmt.Errorf("converting Prometheus object to unstructured failed: %w", err)
}
unstructuredRequiredPrometheus := &unstructured.Unstructured{}
unstructuredRequiredPrometheus.SetUnstructuredContent(unstructuredRequiredPrometheusObject)

// Set the GVK for the unstructured object, since these are not inferred automatically, unlike their structured counterparts.
unstructuredRequiredPrometheus.SetGroupVersionKind(monv1.SchemeGroupVersion.WithKind(monv1.PrometheusesKind))

// Preserve the original behavior: always merge the metadata, never replace.
// Refer: https://github.com/openshift/cluster-monitoring-operator/pull/942.
prometheusGVR := unstructuredRequiredPrometheus.GroupVersionKind().GroupVersion().WithResource(monv1.PrometheusName)
unstructuredExistingPrometheus, err := c.dclient.
Resource(prometheusGVR).
Namespace(structuredRequiredPrometheus.GetNamespace()).
Get(ctx, structuredRequiredPrometheus.GetName(), metav1.GetOptions{})
if apierrors.IsNotFound(err) {
_, err := pclient.Create(ctx, p, metav1.CreateOptions{})
_, didUpdate, err := resourceapply.ApplyUnstructuredResourceImproved(
ctx,
c.dclient,
c.eventRecorder,
unstructuredRequiredPrometheus,
c.resourceCache,
prometheusGVR,
nil,
nil,
)
if err != nil {
return fmt.Errorf("creating Prometheus object failed: %w", err)
return didUpdate, fmt.Errorf("creating Prometheus object failed: %w", err)
}
return nil

return true, nil
}
if err != nil {
return fmt.Errorf("retrieving Prometheus object failed: %w", err)
return false, fmt.Errorf("retrieving Prometheus object failed: %w", err)
}
unstructuredRequiredPrometheusMetadataLabels := unstructuredRequiredPrometheus.GetLabels()
unstructuredRequiredPrometheusMetadataAnnotations := unstructuredRequiredPrometheus.GetAnnotations()
mergeMetadataLabels(unstructuredRequiredPrometheusMetadataLabels, unstructuredExistingPrometheus.GetLabels())
mergeMetadataAnnotations(unstructuredRequiredPrometheusMetadataAnnotations, unstructuredExistingPrometheus.GetAnnotations())
unstructuredRequiredPrometheus.SetLabels(unstructuredRequiredPrometheusMetadataLabels)
unstructuredRequiredPrometheus.SetAnnotations(unstructuredRequiredPrometheusMetadataAnnotations)
unstructuredRequiredPrometheus.SetResourceVersion(unstructuredExistingPrometheus.GetResourceVersion())

required := p.DeepCopy()
mergeMetadata(&required.ObjectMeta, existing.ObjectMeta)

required.ResourceVersion = existing.ResourceVersion
_, err = pclient.Update(ctx, required, metav1.UpdateOptions{})
_, didUpdate, err := resourceapply.ApplyUnstructuredResourceImproved(
ctx,
c.dclient,
c.eventRecorder,
unstructuredRequiredPrometheus,
c.resourceCache,
prometheusGVR,
nil,
nil,
)
if err != nil {
return fmt.Errorf("updating Prometheus object failed: %w", err)
return didUpdate, fmt.Errorf("updating Prometheus object failed: %w", err)
}
return nil

return didUpdate, nil
}

func (c *Client) CreateOrUpdatePrometheusRule(ctx context.Context, p *monv1.PrometheusRule) error {
Expand Down Expand Up @@ -1807,20 +1865,28 @@ func (c *Client) VPACustomResourceDefinitionPresent(ctx context.Context, lastKno
// where keys starting from string defined in `metadataPrefix` are deleted. This prevents issues with preserving stale
// metadata defined by the operator
func mergeMetadata(required *metav1.ObjectMeta, existing metav1.ObjectMeta) {
for k := range existing.Annotations {
mergeMetadataLabels(required.Labels, existing.Labels)
mergeMetadataAnnotations(required.Annotations, existing.Annotations)
}

func mergeMetadataLabels(requiredLabels map[string]string, existingLabels map[string]string) {
for k := range existingLabels {
if strings.HasPrefix(k, metadataPrefix) {
delete(existing.Annotations, k)
delete(existingLabels, k)
}
}

for k := range existing.Labels {
_ = mergo.Merge(&requiredLabels, existingLabels)
}

func mergeMetadataAnnotations(requiredAnnotations map[string]string, existingAnnotations map[string]string) {
for k := range existingAnnotations {
if strings.HasPrefix(k, metadataPrefix) {
delete(existing.Labels, k)
delete(existingAnnotations, k)
}
}

_ = mergo.Merge(&required.Annotations, existing.Annotations)
_ = mergo.Merge(&required.Labels, existing.Labels)
_ = mergo.Merge(&requiredAnnotations, existingAnnotations)
}

type jsonPatch struct {
Expand Down
Loading

0 comments on commit 15b56ea

Please sign in to comment.