From 18cfe7abb12189af3cdd951d74c24c7ae4cc4815 Mon Sep 17 00:00:00 2001 From: Mirza Kopic Date: Mon, 2 Jun 2025 22:08:05 +0200 Subject: [PATCH 1/5] feat: data sink, replaced harcoded secret for credentials --- .gitignore | 1 + Makefile | 18 +- README.md | 109 +++- api/v1alpha1/datasink_types.go | 89 ++++ api/v1alpha1/federatedmanagedmetric_types.go | 6 + api/v1alpha1/federatedmetric_types.go | 6 + api/v1alpha1/managedmetric_types.go | 6 + api/v1alpha1/metric_types.go | 13 + api/v1alpha1/zz_generated.deepcopy.go | 190 ++++++- charts/metrics-operator/values.yaml | 62 ++- .../crds/metrics.cloud.sap_datasinks.yaml | 177 +++++++ ...ics.cloud.sap_federatedmanagedmetrics.yaml | 12 + .../metrics.cloud.sap_federatedmetrics.yaml | 12 + .../metrics.cloud.sap_managedmetrics.yaml | 12 + .../crds/metrics.cloud.sap_metrics.yaml | 12 + .../bases/metrics.cloud.sap_datasinks.yaml | 177 +++++++ ...ics.cloud.sap_federatedmanagedmetrics.yaml | 12 + .../metrics.cloud.sap_federatedmetrics.yaml | 12 + .../metrics.cloud.sap_managedmetrics.yaml | 12 + .../crd/bases/metrics.cloud.sap_metrics.yaml | 12 + config/crd/kustomization.yaml | 1 + config/rbac/datasink_editor_role.yaml | 31 ++ config/rbac/datasink_viewer_role.yaml | 27 + config/rbac/kustomization.yaml | 14 +- config/rbac/role.yaml | 14 + docs/datasink-configuration.md | 483 ++++++++++++++++++ examples/basic_metric.yaml | 19 +- examples/datasink/basic-datasink.yaml | 28 + .../datasink/metric-using-dynatrace-prod.yaml | 14 + examples/dev-core/metric.yaml | 4 + examples/managed_metric.yaml | 4 + .../remoteclusteraccess/metric_with_rca.yaml | 12 + examples/v1beta1/fedmetric.yaml | 4 + internal/clientoptl/optel_test.go | 2 +- internal/clientoptl/optl.go | 28 +- internal/common/common.go | 44 -- internal/controller/datasink_utils.go | 142 +++++ .../federatedmanagedmetric_controller.go | 26 +- .../controller/federatedmetric_controller.go | 41 +- .../controller/managedmetric_controller.go | 23 +- internal/controller/metric_controller.go | 22 +- internal/controller/metric_controller_test.go | 53 +- 42 files changed, 1800 insertions(+), 186 deletions(-) create mode 100644 api/v1alpha1/datasink_types.go create mode 100644 cmd/embedded/crds/metrics.cloud.sap_datasinks.yaml create mode 100644 config/crd/bases/metrics.cloud.sap_datasinks.yaml create mode 100644 config/rbac/datasink_editor_role.yaml create mode 100644 config/rbac/datasink_viewer_role.yaml create mode 100644 docs/datasink-configuration.md create mode 100644 examples/datasink/basic-datasink.yaml create mode 100644 examples/datasink/metric-using-dynatrace-prod.yaml create mode 100644 internal/controller/datasink_utils.go diff --git a/.gitignore b/.gitignore index 61c499d..2d33e1d 100644 --- a/.gitignore +++ b/.gitignore @@ -31,3 +31,4 @@ Dockerfile.cross examples/sample_secret.yaml examples/dynatrace_secret.yaml examples/secret.yaml +/examples/datasink/dynatrace-prod-setup.yaml diff --git a/Makefile b/Makefile index 730476b..f7650ac 100644 --- a/Makefile +++ b/Makefile @@ -79,7 +79,7 @@ build: manifests generate fmt vet ## Build manager binary. .PHONY: run run: manifests generate fmt vet ## Run a controller from your host. - go run ./cmd/main.go start + OPERATOR_CONFIG_NAMESPACE=metrics-operator-system go run ./cmd/main.go start .PHONY: build-docker-binary build-docker-binary: manifests generate fmt vet ## Build manager binary. @@ -238,13 +238,11 @@ dev-local-all: $(MAKE) crossplane-provider-sample $(MAKE) dev-namespace $(MAKE) dev-secret + $(MAKE) dev-operator-namespace $(MAKE) dev-basic-metric $(MAKE) dev-managed-metric - - - .PHONY: dev-secret dev-secret: kubectl apply -f examples/secret.yaml @@ -253,6 +251,10 @@ dev-secret: dev-namespace: kubectl apply -f examples/namespace.yaml +.PHONY: dev-operator-namespace +dev-operator-namespace: + kubectl create namespace metrics-operator-system --dry-run=client -o yaml | kubectl apply -f - + .PHONY: dev-basic-metric dev-basic-metric: kubectl apply -f examples/basic_metric.yaml @@ -261,6 +263,14 @@ dev-basic-metric: dev-managed-metric: kubectl apply -f examples/managed_metric.yaml +.PHONY: dev-apply-dynatrace-prod-setup +dev-apply-dynatrace-prod-setup: + kubectl apply -f examples/datasink/dynatrace-prod-setup.yaml + +.PHONY: dev-apply-metric-dynatrace-prod +dev-apply-metric-dynatrace-prod: + kubectl apply -f examples/datasink/metric-using-dynatrace-prod.yaml + .PHONY: dev-v1beta1-compmetric dev-v1beta1-compmetric: kubectl apply -f examples/v1beta1/compmetric.yaml diff --git a/README.md b/README.md index a0a77cf..aced87e 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,7 @@ The Metrics Operator is a powerful tool designed to monitor and provide insights - [Usage](#usage) - [RBAC Configuration](#rbac-configuration) - [Remote Cluster Access](#remote-cluster-access) +- [DataSink Configuration](#datasink-configuration) - [Data Sink Integration](#data-sink-integration) ## Key Features @@ -124,7 +125,7 @@ graph LR ### Prerequisites 1. Create a namespace for the Metrics Operator. -2. Create a secret containing the data sink credentials in the operator's namespace. +2. Create a DataSink resource and associated authentication secret for your metrics destination. ### Deployment @@ -139,6 +140,8 @@ helm upgrade --install metrics-operator ghcr.io/sap/github.com/sap/metrics-opera Replace `` and `` with appropriate values. +After deployment, create your DataSink configuration as described in the [DataSink Configuration](#datasink-configuration) section. + ## Usage ### Metric @@ -320,13 +323,113 @@ kubectl apply -f rbac-config.yaml Remember to update this RBAC configuration whenever you add new resource types to monitor. +## DataSink Configuration + +The Metrics Operator uses DataSink custom resources to define where and how metrics data should be sent. This provides a flexible and secure way to configure data destinations. + +### Creating a DataSink + +Define a DataSink resource to specify the connection details and authentication for your metrics destination: + +```yaml +apiVersion: metrics.cloud.sap/v1alpha1 +kind: DataSink +metadata: + name: default + namespace: metrics-operator-system +spec: + connection: + endpoint: "https://your-tenant.live.dynatrace.com/api/v2/metrics/ingest" + protocol: "http" + insecureSkipVerify: false + authentication: + apiKey: + secretKeyRef: + name: dynatrace-credentials + key: api-token +``` + +### DataSink Specification + +The `DataSinkSpec` contains the following fields: + +#### Connection +- **endpoint**: The target endpoint URL where metrics will be sent +- **protocol**: Communication protocol (`http` or `grpc`) +- **insecureSkipVerify**: (Optional) Skip TLS certificate verification + +#### Authentication +- **apiKey**: API key authentication configuration + - **secretKeyRef**: Reference to a Kubernetes Secret containing the API key + - **name**: Name of the Secret + - **key**: Key within the Secret containing the API token + +### Using DataSink in Metrics + +All metric types support the `dataSinkRef` field to specify which DataSink to use: + +```yaml +apiVersion: metrics.cloud.sap/v1alpha1 +kind: Metric +metadata: + name: pod-count +spec: + name: "pods.count" + target: + kind: Pod + group: "" + version: v1 + dataSinkRef: + name: default # References the DataSink named "default" +``` + +### Default Behavior + +If no `dataSinkRef` is specified in a metric resource, the operator will automatically use a DataSink named "default" in the operator's namespace. This provides backward compatibility and simplifies configuration for single data sink deployments. + +### Supported Metric Types + +The `dataSinkRef` field is available in all metric resource types: + +- [`Metric`](#metric): Basic metrics for Kubernetes resources +- [`ManagedMetric`](#managed-metric): Metrics for Crossplane managed resources +- [`FederatedMetric`](#federated-metric): Metrics across multiple clusters +- [`FederatedManagedMetric`](#federated-managed-metric): Managed resource metrics across multiple clusters + +### Examples and Detailed Documentation + +For complete examples and more detailed configuration options: + +- See the [`examples/datasink/`](examples/datasink/) directory for practical examples +- Read the comprehensive [DataSink Configuration Guide](docs/datasink-configuration.md) for detailed documentation + +The examples directory contains: +- Basic DataSink configuration examples +- Examples showing DataSink usage with different metric types +- Migration guidance from legacy configurations + +The detailed guide covers: +- Complete specification reference +- Multiple DataSink scenarios +- Advanced configuration options +- Troubleshooting and best practices + +### Migration from Legacy Configuration + +**Important**: The old method of using hardcoded secret names (such as `co-dynatrace-credentials`) has been deprecated and removed. You must now use DataSink resources to configure your metrics destinations. + +To migrate: +1. Create a DataSink resource pointing to your existing authentication secret +2. Update your metric resources to reference the DataSink using `dataSinkRef` +3. Remove any hardcoded secret references from your configuration + ## Data Sink Integration -The Metrics Operator sends collected data to a configured data sink for storage and analysis. The data sink (e.g., Dynatrace) provides tools for data aggregation, filtering, and visualization. +The Metrics Operator sends collected data to configured data sinks for storage and analysis. Data sinks (e.g., Dynatrace) provide tools for data aggregation, filtering, and visualization. To make the most of your metrics: -1. Configure your data sink according to its documentation. +1. Configure your DataSink resources according to your data sink's documentation. 2. Use the data sink's query language or UI to create custom views of your metrics. 3. Set up alerts based on metric thresholds or patterns. 4. Leverage the data sink's analysis tools to gain insights into your system's behavior and performance. diff --git a/api/v1alpha1/datasink_types.go b/api/v1alpha1/datasink_types.go new file mode 100644 index 0000000..d129ff9 --- /dev/null +++ b/api/v1alpha1/datasink_types.go @@ -0,0 +1,89 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// Connection defines the connection details for the DataSink +type Connection struct { + // Endpoint specifies the target endpoint URL + Endpoint string `json:"endpoint"` + // Protocol specifies the communication protocol + // +kubebuilder:validation:Enum=http;grpc + Protocol string `json:"protocol"` + // InsecureSkipVerify controls whether to skip TLS certificate verification + // +optional + InsecureSkipVerify bool `json:"insecureSkipVerify,omitempty"` +} + +// APIKeyAuthentication defines API key authentication configuration +type APIKeyAuthentication struct { + // SecretKeyRef references a key in a Kubernetes Secret containing the API key + SecretKeyRef corev1.SecretKeySelector `json:"secretKeyRef"` +} + +// Authentication defines authentication mechanisms for the DataSink +type Authentication struct { + // APIKey specifies API key authentication configuration + // +optional + APIKey *APIKeyAuthentication `json:"apiKey,omitempty"` +} + +// DataSinkSpec defines the desired state of DataSink +type DataSinkSpec struct { + // Connection specifies the connection details for the data sink + Connection Connection `json:"connection"` + // Authentication specifies the authentication configuration + // +optional + Authentication *Authentication `json:"authentication,omitempty"` +} + +// DataSinkStatus defines the observed state of DataSink +type DataSinkStatus struct { + // Conditions represent the latest available observations of an object's state + // +optional + Conditions []metav1.Condition `json:"conditions,omitempty"` +} + +// DataSink is the Schema for the datasinks API +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status +// +kubebuilder:printcolumn:name="ENDPOINT",type="string",JSONPath=".spec.connection.endpoint" +// +kubebuilder:printcolumn:name="PROTOCOL",type="string",JSONPath=".spec.connection.protocol" +// +kubebuilder:printcolumn:name="AGE",type="date",JSONPath=".metadata.creationTimestamp" +type DataSink struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec DataSinkSpec `json:"spec,omitempty"` + Status DataSinkStatus `json:"status,omitempty"` +} + +// DataSinkList contains a list of DataSink +// +kubebuilder:object:root=true +type DataSinkList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []DataSink `json:"items"` +} + +func init() { + SchemeBuilder.Register(&DataSink{}, &DataSinkList{}) +} diff --git a/api/v1alpha1/federatedmanagedmetric_types.go b/api/v1alpha1/federatedmanagedmetric_types.go index a497727..71deaa0 100644 --- a/api/v1alpha1/federatedmanagedmetric_types.go +++ b/api/v1alpha1/federatedmanagedmetric_types.go @@ -41,6 +41,12 @@ type FederatedManagedMetricSpec struct { // +kubebuilder:default:="10m" Interval metav1.Duration `json:"interval,omitempty"` + // DataSinkRef specifies the DataSink to be used for this federated managed metric. + // If not specified, the DataSink named "default" in the operator's + // namespace will be used. + // +optional + DataSinkRef *DataSinkReference `json:"dataSinkRef,omitempty"` + FederatedClusterAccessRef FederateClusterAccessRef `json:"federateClusterAccessRef,omitempty"` } diff --git a/api/v1alpha1/federatedmetric_types.go b/api/v1alpha1/federatedmetric_types.go index e06fc30..b2feac2 100644 --- a/api/v1alpha1/federatedmetric_types.go +++ b/api/v1alpha1/federatedmetric_types.go @@ -44,6 +44,12 @@ type FederatedMetricSpec struct { // +kubebuilder:default:="10m" Interval metav1.Duration `json:"interval,omitempty"` + // DataSinkRef specifies the DataSink to be used for this federated metric. + // If not specified, the DataSink named "default" in the operator's + // namespace will be used. + // +optional + DataSinkRef *DataSinkReference `json:"dataSinkRef,omitempty"` + FederatedClusterAccessRef FederateClusterAccessRef `json:"federateClusterAccessRef,omitempty"` } diff --git a/api/v1alpha1/managedmetric_types.go b/api/v1alpha1/managedmetric_types.go index 1f20c5d..6a0a4ba 100644 --- a/api/v1alpha1/managedmetric_types.go +++ b/api/v1alpha1/managedmetric_types.go @@ -46,6 +46,12 @@ type ManagedMetricSpec struct { // +kubebuilder:default:="10m" Interval metav1.Duration `json:"interval,omitempty"` + // DataSinkRef specifies the DataSink to be used for this managed metric. + // If not specified, the DataSink named "default" in the operator's + // namespace will be used. + // +optional + DataSinkRef *DataSinkReference `json:"dataSinkRef,omitempty"` + // +optional *RemoteClusterAccessRef `json:"remoteClusterAccessRef,omitempty"` } diff --git a/api/v1alpha1/metric_types.go b/api/v1alpha1/metric_types.go index 93be433..1f6f575 100644 --- a/api/v1alpha1/metric_types.go +++ b/api/v1alpha1/metric_types.go @@ -36,6 +36,13 @@ const ( PhasePending PhaseType = "Pending" ) +// DataSinkReference holds a reference to a DataSink resource. +type DataSinkReference struct { + // Name is the name of the DataSink resource. + // +kubebuilder:validation:Required + Name string `json:"name"` +} + // MetricSpec defines the desired state of Metric type MetricSpec struct { // Sets the name that will be used to identify the metric in Dynatrace(or other providers) @@ -55,6 +62,12 @@ type MetricSpec struct { // +kubebuilder:default:="10m" Interval metav1.Duration `json:"interval,omitempty"` + // DataSinkRef specifies the DataSink to be used for this metric. + // If not specified, the DataSink named "default" in the operator's + // namespace will be used. + // +optional + DataSinkRef *DataSinkReference `json:"dataSinkRef,omitempty"` + // +optional *RemoteClusterAccessRef `json:"remoteClusterAccessRef,omitempty"` diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 12776f8..f6ea205 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -25,6 +25,42 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *APIKeyAuthentication) DeepCopyInto(out *APIKeyAuthentication) { + *out = *in + in.SecretKeyRef.DeepCopyInto(&out.SecretKeyRef) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new APIKeyAuthentication. +func (in *APIKeyAuthentication) DeepCopy() *APIKeyAuthentication { + if in == nil { + return nil + } + out := new(APIKeyAuthentication) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Authentication) DeepCopyInto(out *Authentication) { + *out = *in + if in.APIKey != nil { + in, out := &in.APIKey, &out.APIKey + *out = new(APIKeyAuthentication) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Authentication. +func (in *Authentication) DeepCopy() *Authentication { + if in == nil { + return nil + } + out := new(Authentication) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterAccessConfig) DeepCopyInto(out *ClusterAccessConfig) { *out = *in @@ -41,6 +77,138 @@ func (in *ClusterAccessConfig) DeepCopy() *ClusterAccessConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Connection) DeepCopyInto(out *Connection) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Connection. +func (in *Connection) DeepCopy() *Connection { + if in == nil { + return nil + } + out := new(Connection) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DataSink) DeepCopyInto(out *DataSink) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DataSink. +func (in *DataSink) DeepCopy() *DataSink { + if in == nil { + return nil + } + out := new(DataSink) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *DataSink) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DataSinkList) DeepCopyInto(out *DataSinkList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]DataSink, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DataSinkList. +func (in *DataSinkList) DeepCopy() *DataSinkList { + if in == nil { + return nil + } + out := new(DataSinkList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *DataSinkList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DataSinkReference) DeepCopyInto(out *DataSinkReference) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DataSinkReference. +func (in *DataSinkReference) DeepCopy() *DataSinkReference { + if in == nil { + return nil + } + out := new(DataSinkReference) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DataSinkSpec) DeepCopyInto(out *DataSinkSpec) { + *out = *in + out.Connection = in.Connection + if in.Authentication != nil { + in, out := &in.Authentication, &out.Authentication + *out = new(Authentication) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DataSinkSpec. +func (in *DataSinkSpec) DeepCopy() *DataSinkSpec { + if in == nil { + return nil + } + out := new(DataSinkSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DataSinkStatus) DeepCopyInto(out *DataSinkStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DataSinkStatus. +func (in *DataSinkStatus) DeepCopy() *DataSinkStatus { + if in == nil { + return nil + } + out := new(DataSinkStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Dimension) DeepCopyInto(out *Dimension) { *out = *in @@ -166,7 +334,7 @@ func (in *FederatedManagedMetric) DeepCopyInto(out *FederatedManagedMetric) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) } @@ -224,6 +392,11 @@ func (in *FederatedManagedMetricList) DeepCopyObject() runtime.Object { func (in *FederatedManagedMetricSpec) DeepCopyInto(out *FederatedManagedMetricSpec) { *out = *in out.Interval = in.Interval + if in.DataSinkRef != nil { + in, out := &in.DataSinkRef, &out.DataSinkRef + *out = new(DataSinkReference) + **out = **in + } out.FederatedClusterAccessRef = in.FederatedClusterAccessRef } @@ -333,6 +506,11 @@ func (in *FederatedMetricSpec) DeepCopyInto(out *FederatedMetricSpec) { copy(*out, *in) } out.Interval = in.Interval + if in.DataSinkRef != nil { + in, out := &in.DataSinkRef, &out.DataSinkRef + *out = new(DataSinkReference) + **out = **in + } out.FederatedClusterAccessRef = in.FederatedClusterAccessRef } @@ -481,6 +659,11 @@ func (in *ManagedMetricList) DeepCopyObject() runtime.Object { func (in *ManagedMetricSpec) DeepCopyInto(out *ManagedMetricSpec) { *out = *in out.Interval = in.Interval + if in.DataSinkRef != nil { + in, out := &in.DataSinkRef, &out.DataSinkRef + *out = new(DataSinkReference) + **out = **in + } if in.RemoteClusterAccessRef != nil { in, out := &in.RemoteClusterAccessRef, &out.RemoteClusterAccessRef *out = new(RemoteClusterAccessRef) @@ -622,6 +805,11 @@ func (in *MetricSpec) DeepCopyInto(out *MetricSpec) { *out = *in out.Target = in.Target out.Interval = in.Interval + if in.DataSinkRef != nil { + in, out := &in.DataSinkRef, &out.DataSinkRef + *out = new(DataSinkReference) + **out = **in + } if in.RemoteClusterAccessRef != nil { in, out := &in.RemoteClusterAccessRef, &out.RemoteClusterAccessRef *out = new(RemoteClusterAccessRef) diff --git a/charts/metrics-operator/values.yaml b/charts/metrics-operator/values.yaml index d61a872..4606f0e 100644 --- a/charts/metrics-operator/values.yaml +++ b/charts/metrics-operator/values.yaml @@ -108,25 +108,67 @@ rbac: clusterRole: rules: - apiGroups: - - metrics.cloud.sap + - "" resources: - - managedmetrics - - metrics - - metrics/status - - managedmetrics/status + - secrets verbs: - - "*" + - get + - list + - watch - apiGroups: - "" resources: - - secrets + - events + verbs: + - create + - patch + - apiGroups: + - metrics.cloud.sap + resources: + - clusteraccesses + - datasinks + - federatedclusteraccesses + - federatedmanagedmetrics + - federatedmetrics + - managedmetrics + - metrics + - remoteclusteraccesses verbs: + - create + - delete - get - list + - patch + - update - watch - - apiGroups: [ "*" ] - resources: [ "*" ] - verbs: [ "get", "list", "watch" ] + - apiGroups: + - metrics.cloud.sap + resources: + - clusteraccesses/finalizers + - datasinks/finalizers + - federatedclusteraccesses/finalizers + - federatedmanagedmetrics/finalizers + - federatedmetrics/finalizers + - managedmetrics/finalizers + - metrics/finalizers + - remoteclusteraccesses/finalizers + verbs: + - update + - apiGroups: + - metrics.cloud.sap + resources: + - clusteraccesses/status + - datasinks/status + - federatedclusteraccesses/status + - federatedmanagedmetrics/status + - federatedmetrics/status + - managedmetrics/status + - metrics/status + - remoteclusteraccesses/status + verbs: + - get + - patch + - update role: rules: [] diff --git a/cmd/embedded/crds/metrics.cloud.sap_datasinks.yaml b/cmd/embedded/crds/metrics.cloud.sap_datasinks.yaml new file mode 100644 index 0000000..96a5946 --- /dev/null +++ b/cmd/embedded/crds/metrics.cloud.sap_datasinks.yaml @@ -0,0 +1,177 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.17.2 + name: datasinks.metrics.cloud.sap +spec: + group: metrics.cloud.sap + names: + kind: DataSink + listKind: DataSinkList + plural: datasinks + singular: datasink + scope: Namespaced + versions: + - additionalPrinterColumns: + - jsonPath: .spec.connection.endpoint + name: ENDPOINT + type: string + - jsonPath: .spec.connection.protocol + name: PROTOCOL + type: string + - jsonPath: .metadata.creationTimestamp + name: AGE + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: DataSink is the Schema for the datasinks API + 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 requests to. + 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: DataSinkSpec defines the desired state of DataSink + properties: + authentication: + description: Authentication specifies the authentication configuration + properties: + apiKey: + description: APIKey specifies API key authentication configuration + properties: + secretKeyRef: + description: SecretKeyRef references a key in a Kubernetes + Secret containing the API key + properties: + key: + description: The key of the secret to select from. Must + be a valid secret key. + type: string + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + optional: + description: Specify whether the Secret or its key must + be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + required: + - secretKeyRef + type: object + type: object + connection: + description: Connection specifies the connection details for the data + sink + properties: + endpoint: + description: Endpoint specifies the target endpoint URL + type: string + insecureSkipVerify: + description: InsecureSkipVerify controls whether to skip TLS certificate + verification + type: boolean + protocol: + description: Protocol specifies the communication protocol + enum: + - http + - grpc + type: string + required: + - endpoint + - protocol + type: object + required: + - connection + type: object + status: + description: DataSinkStatus defines the observed state of DataSink + properties: + conditions: + description: Conditions represent the latest available observations + of an object's state + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + type: object + type: object + served: true + storage: true + subresources: + status: {} diff --git a/cmd/embedded/crds/metrics.cloud.sap_federatedmanagedmetrics.yaml b/cmd/embedded/crds/metrics.cloud.sap_federatedmanagedmetrics.yaml index d2f1e9a..da337ec 100644 --- a/cmd/embedded/crds/metrics.cloud.sap_federatedmanagedmetrics.yaml +++ b/cmd/embedded/crds/metrics.cloud.sap_federatedmanagedmetrics.yaml @@ -40,6 +40,18 @@ spec: spec: description: FederatedManagedMetricSpec defines the desired state of FederatedManagedMetric properties: + dataSinkRef: + description: |- + DataSinkRef specifies the DataSink to be used for this federated managed metric. + If not specified, the DataSink named "default" in the operator's + namespace will be used. + properties: + name: + description: Name is the name of the DataSink resource. + type: string + required: + - name + type: object description: type: string federateClusterAccessRef: diff --git a/cmd/embedded/crds/metrics.cloud.sap_federatedmetrics.yaml b/cmd/embedded/crds/metrics.cloud.sap_federatedmetrics.yaml index 2c7b1de..76ba266 100644 --- a/cmd/embedded/crds/metrics.cloud.sap_federatedmetrics.yaml +++ b/cmd/embedded/crds/metrics.cloud.sap_federatedmetrics.yaml @@ -39,6 +39,18 @@ spec: spec: description: FederatedMetricSpec defines the desired state of FederatedMetric properties: + dataSinkRef: + description: |- + DataSinkRef specifies the DataSink to be used for this federated metric. + If not specified, the DataSink named "default" in the operator's + namespace will be used. + properties: + name: + description: Name is the name of the DataSink resource. + type: string + required: + - name + type: object description: type: string federateClusterAccessRef: diff --git a/cmd/embedded/crds/metrics.cloud.sap_managedmetrics.yaml b/cmd/embedded/crds/metrics.cloud.sap_managedmetrics.yaml index ee02398..3c3c554 100644 --- a/cmd/embedded/crds/metrics.cloud.sap_managedmetrics.yaml +++ b/cmd/embedded/crds/metrics.cloud.sap_managedmetrics.yaml @@ -49,6 +49,18 @@ spec: spec: description: ManagedMetricSpec defines the desired state of ManagedMetric properties: + dataSinkRef: + description: |- + DataSinkRef specifies the DataSink to be used for this managed metric. + If not specified, the DataSink named "default" in the operator's + namespace will be used. + properties: + name: + description: Name is the name of the DataSink resource. + type: string + required: + - name + type: object description: description: Sets the description that will be used to identify the metric in Dynatrace(or other providers) diff --git a/cmd/embedded/crds/metrics.cloud.sap_metrics.yaml b/cmd/embedded/crds/metrics.cloud.sap_metrics.yaml index 0cbf66e..eef844f 100644 --- a/cmd/embedded/crds/metrics.cloud.sap_metrics.yaml +++ b/cmd/embedded/crds/metrics.cloud.sap_metrics.yaml @@ -49,6 +49,18 @@ spec: spec: description: MetricSpec defines the desired state of Metric properties: + dataSinkRef: + description: |- + DataSinkRef specifies the DataSink to be used for this metric. + If not specified, the DataSink named "default" in the operator's + namespace will be used. + properties: + name: + description: Name is the name of the DataSink resource. + type: string + required: + - name + type: object description: description: Sets the description that will be used to identify the metric in Dynatrace(or other providers) diff --git a/config/crd/bases/metrics.cloud.sap_datasinks.yaml b/config/crd/bases/metrics.cloud.sap_datasinks.yaml new file mode 100644 index 0000000..96a5946 --- /dev/null +++ b/config/crd/bases/metrics.cloud.sap_datasinks.yaml @@ -0,0 +1,177 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.17.2 + name: datasinks.metrics.cloud.sap +spec: + group: metrics.cloud.sap + names: + kind: DataSink + listKind: DataSinkList + plural: datasinks + singular: datasink + scope: Namespaced + versions: + - additionalPrinterColumns: + - jsonPath: .spec.connection.endpoint + name: ENDPOINT + type: string + - jsonPath: .spec.connection.protocol + name: PROTOCOL + type: string + - jsonPath: .metadata.creationTimestamp + name: AGE + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: DataSink is the Schema for the datasinks API + 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 requests to. + 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: DataSinkSpec defines the desired state of DataSink + properties: + authentication: + description: Authentication specifies the authentication configuration + properties: + apiKey: + description: APIKey specifies API key authentication configuration + properties: + secretKeyRef: + description: SecretKeyRef references a key in a Kubernetes + Secret containing the API key + properties: + key: + description: The key of the secret to select from. Must + be a valid secret key. + type: string + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + optional: + description: Specify whether the Secret or its key must + be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + required: + - secretKeyRef + type: object + type: object + connection: + description: Connection specifies the connection details for the data + sink + properties: + endpoint: + description: Endpoint specifies the target endpoint URL + type: string + insecureSkipVerify: + description: InsecureSkipVerify controls whether to skip TLS certificate + verification + type: boolean + protocol: + description: Protocol specifies the communication protocol + enum: + - http + - grpc + type: string + required: + - endpoint + - protocol + type: object + required: + - connection + type: object + status: + description: DataSinkStatus defines the observed state of DataSink + properties: + conditions: + description: Conditions represent the latest available observations + of an object's state + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + type: object + type: object + served: true + storage: true + subresources: + status: {} diff --git a/config/crd/bases/metrics.cloud.sap_federatedmanagedmetrics.yaml b/config/crd/bases/metrics.cloud.sap_federatedmanagedmetrics.yaml index d2f1e9a..da337ec 100644 --- a/config/crd/bases/metrics.cloud.sap_federatedmanagedmetrics.yaml +++ b/config/crd/bases/metrics.cloud.sap_federatedmanagedmetrics.yaml @@ -40,6 +40,18 @@ spec: spec: description: FederatedManagedMetricSpec defines the desired state of FederatedManagedMetric properties: + dataSinkRef: + description: |- + DataSinkRef specifies the DataSink to be used for this federated managed metric. + If not specified, the DataSink named "default" in the operator's + namespace will be used. + properties: + name: + description: Name is the name of the DataSink resource. + type: string + required: + - name + type: object description: type: string federateClusterAccessRef: diff --git a/config/crd/bases/metrics.cloud.sap_federatedmetrics.yaml b/config/crd/bases/metrics.cloud.sap_federatedmetrics.yaml index 2c7b1de..76ba266 100644 --- a/config/crd/bases/metrics.cloud.sap_federatedmetrics.yaml +++ b/config/crd/bases/metrics.cloud.sap_federatedmetrics.yaml @@ -39,6 +39,18 @@ spec: spec: description: FederatedMetricSpec defines the desired state of FederatedMetric properties: + dataSinkRef: + description: |- + DataSinkRef specifies the DataSink to be used for this federated metric. + If not specified, the DataSink named "default" in the operator's + namespace will be used. + properties: + name: + description: Name is the name of the DataSink resource. + type: string + required: + - name + type: object description: type: string federateClusterAccessRef: diff --git a/config/crd/bases/metrics.cloud.sap_managedmetrics.yaml b/config/crd/bases/metrics.cloud.sap_managedmetrics.yaml index ee02398..3c3c554 100644 --- a/config/crd/bases/metrics.cloud.sap_managedmetrics.yaml +++ b/config/crd/bases/metrics.cloud.sap_managedmetrics.yaml @@ -49,6 +49,18 @@ spec: spec: description: ManagedMetricSpec defines the desired state of ManagedMetric properties: + dataSinkRef: + description: |- + DataSinkRef specifies the DataSink to be used for this managed metric. + If not specified, the DataSink named "default" in the operator's + namespace will be used. + properties: + name: + description: Name is the name of the DataSink resource. + type: string + required: + - name + type: object description: description: Sets the description that will be used to identify the metric in Dynatrace(or other providers) diff --git a/config/crd/bases/metrics.cloud.sap_metrics.yaml b/config/crd/bases/metrics.cloud.sap_metrics.yaml index 0cbf66e..eef844f 100644 --- a/config/crd/bases/metrics.cloud.sap_metrics.yaml +++ b/config/crd/bases/metrics.cloud.sap_metrics.yaml @@ -49,6 +49,18 @@ spec: spec: description: MetricSpec defines the desired state of Metric properties: + dataSinkRef: + description: |- + DataSinkRef specifies the DataSink to be used for this metric. + If not specified, the DataSink named "default" in the operator's + namespace will be used. + properties: + name: + description: Name is the name of the DataSink resource. + type: string + required: + - name + type: object description: description: Sets the description that will be used to identify the metric in Dynatrace(or other providers) diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 056b904..2e78881 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -4,6 +4,7 @@ resources: - bases/metrics.cloud.sap_metrics.yaml - bases/metrics.cloud.sap_managedmetrics.yaml +- bases/metrics.cloud.sap_datasinks.yaml - bases/metrics.cloud.sap_remoteclusteraccesses.yaml - bases/metrics.cloud.sap_federatedmetrics.yaml - bases/metrics.cloud.sap_clusteraccesses.yaml diff --git a/config/rbac/datasink_editor_role.yaml b/config/rbac/datasink_editor_role.yaml new file mode 100644 index 0000000..7a8987c --- /dev/null +++ b/config/rbac/datasink_editor_role.yaml @@ -0,0 +1,31 @@ +# permissions for end users to edit datasinks. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + labels: + app.kubernetes.io/name: clusterrole + app.kubernetes.io/instance: datasink-editor-role + app.kubernetes.io/component: rbac + app.kubernetes.io/created-by: co-metrics-operator + app.kubernetes.io/part-of: co-metrics-operator + app.kubernetes.io/managed-by: kustomize + name: datasink-editor-role +rules: +- apiGroups: + - metrics.cloud.sap + resources: + - datasinks + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - metrics.cloud.sap + resources: + - datasinks/status + verbs: + - get \ No newline at end of file diff --git a/config/rbac/datasink_viewer_role.yaml b/config/rbac/datasink_viewer_role.yaml new file mode 100644 index 0000000..e02999c --- /dev/null +++ b/config/rbac/datasink_viewer_role.yaml @@ -0,0 +1,27 @@ +# permissions for end users to view datasinks. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + labels: + app.kubernetes.io/name: clusterrole + app.kubernetes.io/instance: datasink-viewer-role + app.kubernetes.io/component: rbac + app.kubernetes.io/created-by: co-metrics-operator + app.kubernetes.io/part-of: co-metrics-operator + app.kubernetes.io/managed-by: kustomize + name: datasink-viewer-role +rules: +- apiGroups: + - metrics.cloud.sap + resources: + - datasinks + verbs: + - get + - list + - watch +- apiGroups: + - metrics.cloud.sap + resources: + - datasinks/status + verbs: + - get \ No newline at end of file diff --git a/config/rbac/kustomization.yaml b/config/rbac/kustomization.yaml index c55f809..c42b743 100644 --- a/config/rbac/kustomization.yaml +++ b/config/rbac/kustomization.yaml @@ -20,14 +20,20 @@ resources: # default, aiding admins in cluster management. Those roles are # not used by the Project itself. You can comment the following lines # if you do not want those helpers be installed with your Project. -- federatedmanagedmetric_editor_role.yaml -- federatedmanagedmetric_viewer_role.yaml -- federatedclusteraccess_editor_role.yaml -- federatedclusteraccess_viewer_role.yaml - clusteraccess_editor_role.yaml - clusteraccess_viewer_role.yaml +- datasink_editor_role.yaml +- datasink_viewer_role.yaml +- federatedclusteraccess_editor_role.yaml +- federatedclusteraccess_viewer_role.yaml +- federatedmanagedmetric_editor_role.yaml +- federatedmanagedmetric_viewer_role.yaml - federatedmetric_editor_role.yaml - federatedmetric_viewer_role.yaml +- managedmetric_editor_role.yaml +- managedmetric_viewer_role.yaml +- metric_editor_role.yaml +- metric_viewer_role.yaml - remoteclusteraccess_editor_role.yaml - remoteclusteraccess_viewer_role.yaml diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 121721e..74ab89e 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -4,6 +4,12 @@ kind: ClusterRole metadata: name: manager-role rules: +- apiGroups: + - "" + resources: + - secrets + verbs: + - get - apiGroups: - metrics.cloud.sap resources: @@ -39,3 +45,11 @@ rules: - get - patch - update +- apiGroups: + - metrics.cloud.sap + resources: + - datasinks + verbs: + - get + - list + - watch diff --git a/docs/datasink-configuration.md b/docs/datasink-configuration.md new file mode 100644 index 0000000..e99a4f6 --- /dev/null +++ b/docs/datasink-configuration.md @@ -0,0 +1,483 @@ +# DataSink Configuration Guide + +This guide provides comprehensive information about configuring and using DataSink custom resources with the Metrics Operator. + +## Overview + +DataSink is a custom resource that defines where and how the Metrics Operator should send collected metrics data. It provides a flexible, secure, and centralized way to configure data destinations, replacing the previous hardcoded secret approach. + +## DataSink Resource Structure + +### Complete Example + +```yaml +apiVersion: metrics.cloud.sap/v1alpha1 +kind: DataSink +metadata: + name: production-dynatrace + namespace: metrics-operator-system + labels: + environment: production + provider: dynatrace +spec: + connection: + endpoint: "https://abc12345.live.dynatrace.com/api/v2/metrics/ingest" + protocol: "http" + insecureSkipVerify: false + authentication: + apiKey: + secretKeyRef: + name: dynatrace-production-credentials + key: api-token +``` + +### Specification Fields + +#### `spec.connection` + +Defines the connection details for the data sink. + +- **`endpoint`** (required): The target endpoint URL where metrics will be sent + - For Dynatrace: `https://{your-environment-id}.live.dynatrace.com/api/v2/metrics/ingest` + - For custom endpoints: Any valid HTTP/HTTPS or gRPC URL + +- **`protocol`** (required): Communication protocol + - `http`: HTTP/HTTPS protocol + - `grpc`: gRPC protocol + +- **`insecureSkipVerify`** (optional): Skip TLS certificate verification + - `false` (default): Verify TLS certificates + - `true`: Skip TLS verification (not recommended for production) + +#### `spec.authentication` + +Defines authentication mechanisms for the data sink. + +##### API Key Authentication + +Currently, the only supported authentication method is API key authentication: + +- **`apiKey.secretKeyRef`** (required): Reference to a Kubernetes Secret containing the API key + - **`name`**: Name of the Secret containing the API key + - **`key`**: Key within the Secret that contains the API token + +## Secret Configuration + +The authentication secret must be created in the same namespace as the DataSink resource (typically the operator's namespace). + +### Example Secret + +```yaml +apiVersion: v1 +kind: Secret +metadata: + name: dynatrace-production-credentials + namespace: metrics-operator-system +type: Opaque +data: + # Base64 encoded API token + api-token: ZHQwYzAxLmFiY2RlZmdoaWprbG1ub3BxcnN0dXZ3eHl6MTIzNDU2Nzg5MA== +stringData: + # Alternative: provide the token directly (will be base64 encoded automatically) + # api-token: "dt0c01.abcdefghijklmnopqrstuvwxyz1234567890" +``` + +### Creating Secrets + +You can create the secret using `kubectl`: + +```bash +# Create secret with base64 encoding +kubectl create secret generic dynatrace-production-credentials \ + --from-literal=api-token="dt0c01.abcdefghijklmnopqrstuvwxyz1234567890" \ + --namespace=metrics-operator-system + +# Or create from a file +echo -n "dt0c01.abcdefghijklmnopqrstuvwxyz1234567890" > /tmp/token +kubectl create secret generic dynatrace-production-credentials \ + --from-file=api-token=/tmp/token \ + --namespace=metrics-operator-system +``` + +## Using DataSink in Metrics + +### Explicit Reference + +Specify the DataSink to use in your metric resources: + +```yaml +apiVersion: metrics.cloud.sap/v1alpha1 +kind: Metric +metadata: + name: pod-metrics + namespace: default +spec: + name: "kubernetes.pods.count" + description: "Number of pods in the cluster" + target: + kind: Pod + group: "" + version: v1 + interval: "1m" + dataSinkRef: + name: production-dynatrace # References the DataSink +``` + +### Default DataSink + +If no `dataSinkRef` is specified, the operator automatically uses a DataSink named "default" in the operator's namespace: + +```yaml +apiVersion: metrics.cloud.sap/v1alpha1 +kind: Metric +metadata: + name: pod-metrics-default + namespace: default +spec: + name: "kubernetes.pods.count.default" + target: + kind: Pod + group: "" + version: v1 + # No dataSinkRef - uses "default" DataSink automatically +``` + +## Multiple DataSinks + +You can configure multiple DataSinks for different purposes: + +### Environment-based DataSinks + +```yaml +--- +# Development environment DataSink +apiVersion: metrics.cloud.sap/v1alpha1 +kind: DataSink +metadata: + name: dev-dynatrace + namespace: metrics-operator-system +spec: + connection: + endpoint: "https://dev123.live.dynatrace.com/api/v2/metrics/ingest" + protocol: "http" + authentication: + apiKey: + secretKeyRef: + name: dynatrace-dev-credentials + key: api-token +--- +# Production environment DataSink +apiVersion: metrics.cloud.sap/v1alpha1 +kind: DataSink +metadata: + name: prod-dynatrace + namespace: metrics-operator-system +spec: + connection: + endpoint: "https://prod456.live.dynatrace.com/api/v2/metrics/ingest" + protocol: "http" + authentication: + apiKey: + secretKeyRef: + name: dynatrace-prod-credentials + key: api-token +``` + +### Team-based DataSinks + +```yaml +--- +# Platform team DataSink +apiVersion: metrics.cloud.sap/v1alpha1 +kind: DataSink +metadata: + name: platform-metrics + namespace: metrics-operator-system +spec: + connection: + endpoint: "https://platform.live.dynatrace.com/api/v2/metrics/ingest" + protocol: "http" + authentication: + apiKey: + secretKeyRef: + name: platform-dynatrace-credentials + key: api-token +--- +# Application team DataSink +apiVersion: metrics.cloud.sap/v1alpha1 +kind: DataSink +metadata: + name: app-metrics + namespace: metrics-operator-system +spec: + connection: + endpoint: "https://apps.live.dynatrace.com/api/v2/metrics/ingest" + protocol: "http" + authentication: + apiKey: + secretKeyRef: + name: app-dynatrace-credentials + key: api-token +``` + +## Supported Metric Types + +All metric resource types support the `dataSinkRef` field: + +### Metric + +```yaml +apiVersion: metrics.cloud.sap/v1alpha1 +kind: Metric +metadata: + name: service-count +spec: + name: "kubernetes.services.count" + target: + kind: Service + group: "" + version: v1 + dataSinkRef: + name: platform-metrics +``` + +### ManagedMetric + +```yaml +apiVersion: metrics.cloud.sap/v1alpha1 +kind: ManagedMetric +metadata: + name: crossplane-releases +spec: + name: "crossplane.releases.count" + kind: Release + group: helm.crossplane.io + version: v1beta1 + dataSinkRef: + name: platform-metrics +``` + +### FederatedMetric + +```yaml +apiVersion: metrics.cloud.sap/v1alpha1 +kind: FederatedMetric +metadata: + name: federated-providers +spec: + name: "crossplane.providers.federated" + target: + kind: Provider + group: pkg.crossplane.io + version: v1 + federateClusterAccessRef: + name: cluster-federation + namespace: default + dataSinkRef: + name: platform-metrics +``` + +### FederatedManagedMetric + +```yaml +apiVersion: metrics.cloud.sap/v1alpha1 +kind: FederatedManagedMetric +metadata: + name: federated-managed-resources +spec: + name: "crossplane.managed.federated" + federateClusterAccessRef: + name: cluster-federation + namespace: default + dataSinkRef: + name: platform-metrics +``` + +## Migration from Legacy Configuration + +### Before (Deprecated) + +Previously, the operator used hardcoded secret names: + +```yaml +# This approach is no longer supported +apiVersion: v1 +kind: Secret +metadata: + name: co-dynatrace-credentials # Hardcoded name + namespace: co-metrics-operator # Hardcoded namespace +type: Opaque +data: + api-token: +``` + +### After (Current) + +Now you must use DataSink resources: + +```yaml +--- +# 1. Create the DataSink +apiVersion: metrics.cloud.sap/v1alpha1 +kind: DataSink +metadata: + name: default + namespace: metrics-operator-system +spec: + connection: + endpoint: "https://your-tenant.live.dynatrace.com/api/v2/metrics/ingest" + protocol: "http" + authentication: + apiKey: + secretKeyRef: + name: dynatrace-credentials # Any name you choose + key: api-token +--- +# 2. Create the secret with your chosen name +apiVersion: v1 +kind: Secret +metadata: + name: dynatrace-credentials + namespace: metrics-operator-system +type: Opaque +data: + api-token: +``` + +### Migration Steps + +1. **Create DataSink resources** for each data destination you need +2. **Update metric resources** to reference the appropriate DataSink using `dataSinkRef` +3. **Remove old hardcoded secrets** (they are no longer used) +4. **Test your configuration** to ensure metrics are being sent correctly + +## Troubleshooting + +### Common Issues + +#### DataSink Not Found + +**Error**: `DataSink "default" not found in namespace "metrics-operator-system"` + +**Solution**: Create a DataSink named "default" in the operator's namespace, or specify a `dataSinkRef` in your metric resources. + +#### Secret Not Found + +**Error**: `Secret "dynatrace-credentials" not found` + +**Solution**: Ensure the secret referenced in `authentication.apiKey.secretKeyRef` exists in the same namespace as the DataSink. + +#### Authentication Failed + +**Error**: `401 Unauthorized` or similar authentication errors + +**Solution**: +- Verify the API token is correct and has the necessary permissions +- Check that the token is properly base64 encoded in the secret +- Ensure the endpoint URL is correct for your data sink + +#### Connection Failed + +**Error**: `connection refused` or `timeout` errors + +**Solution**: +- Verify the endpoint URL is correct and accessible +- Check network connectivity from the cluster to the data sink +- Verify firewall rules allow outbound connections + +### Debugging Commands + +```bash +# Check DataSink resources +kubectl get datasinks -n metrics-operator-system + +# Describe a specific DataSink +kubectl describe datasink default -n metrics-operator-system + +# Check DataSink status +kubectl get datasink default -n metrics-operator-system -o yaml + +# Check secrets +kubectl get secrets -n metrics-operator-system + +# Check operator logs +kubectl logs -n metrics-operator-system deployment/metrics-operator-controller-manager +``` + +### Validation + +To verify your DataSink configuration is working: + +1. **Check DataSink status**: Look for any error conditions in the DataSink status +2. **Monitor operator logs**: Check for connection or authentication errors +3. **Verify metric delivery**: Confirm metrics are appearing in your data sink +4. **Test connectivity**: Use tools like `curl` to test endpoint connectivity from within the cluster + +## Best Practices + +### Security + +- **Use separate secrets** for different environments (dev, staging, production) +- **Rotate API tokens** regularly and update secrets accordingly +- **Use RBAC** to limit access to DataSink resources and secrets +- **Enable TLS verification** (`insecureSkipVerify: false`) in production + +### Organization + +- **Use descriptive names** for DataSinks (e.g., `production-dynatrace`, `dev-prometheus`) +- **Add labels** to DataSinks for better organization and filtering +- **Document your configuration** with annotations or external documentation +- **Use namespaces** to separate DataSinks by team or environment + +### Monitoring + +- **Monitor DataSink status** for connection issues +- **Set up alerts** for failed metric deliveries +- **Track metric volume** to ensure data is being sent as expected +- **Monitor operator health** to catch configuration issues early + +## Advanced Configuration + +### Custom Endpoints + +DataSink supports any HTTP or gRPC endpoint: + +```yaml +apiVersion: metrics.cloud.sap/v1alpha1 +kind: DataSink +metadata: + name: custom-endpoint + namespace: metrics-operator-system +spec: + connection: + endpoint: "https://custom-metrics.example.com/api/v1/metrics" + protocol: "http" + authentication: + apiKey: + secretKeyRef: + name: custom-api-credentials + key: token +``` + +### gRPC Configuration + +For gRPC endpoints: + +```yaml +apiVersion: metrics.cloud.sap/v1alpha1 +kind: DataSink +metadata: + name: grpc-endpoint + namespace: metrics-operator-system +spec: + connection: + endpoint: "grpc://metrics-service.example.com:9090" + protocol: "grpc" + authentication: + apiKey: + secretKeyRef: + name: grpc-credentials + key: api-key +``` + +This comprehensive guide should help you configure and use DataSink resources effectively with the Metrics Operator. \ No newline at end of file diff --git a/examples/basic_metric.yaml b/examples/basic_metric.yaml index bc33cb5..b65e81d 100644 --- a/examples/basic_metric.yaml +++ b/examples/basic_metric.yaml @@ -10,6 +10,10 @@ spec: group: helm.crossplane.io version: v1beta1 interval: 1m # in minutes + # Uses default DataSink (named "default" in metrics-operator-system namespace) + # To use a custom DataSink, uncomment and specify: + # dataSinkRef: + # name: default --- apiVersion: metrics.cloud.sap/v1alpha1 kind: Metric @@ -39,18 +43,3 @@ spec: group: "" version: v1 interval: 1m # in minutes - -#apiVersion: metrics.cloud.sap/v1alpha1 -#kind: Metric -#metadata: -# name: ext-sa-metric -#spec: -# name: ext-subaccount-metric -# description: Subaccounts in mirza mcp cluster -# kind: Subaccount -# group: account.btp.orchestrate.cloud.sap -# version: v1alpha1 -# frequency: 1 # in minutes -# remoteClusterAccessRef: -# name: remote-cluster-access -# namespace: default diff --git a/examples/datasink/basic-datasink.yaml b/examples/datasink/basic-datasink.yaml new file mode 100644 index 0000000..e207f21 --- /dev/null +++ b/examples/datasink/basic-datasink.yaml @@ -0,0 +1,28 @@ +# Basic DataSink configuration for Dynatrace +apiVersion: metrics.cloud.sap/v1alpha1 +kind: DataSink +metadata: + name: default + namespace: metrics-operator-system +spec: + connection: + endpoint: "https://your-tenant.live.dynatrace.com/api/v2/metrics/ingest/otlp/v1/metrics" + protocol: "http" + insecureSkipVerify: false + authentication: + apiKey: + secretKeyRef: + name: dynatrace-credentials + key: api-token +--- +# Secret containing the Dynatrace API token +apiVersion: v1 +kind: Secret +metadata: + name: dynatrace-credentials + namespace: metrics-operator-system +type: Opaque +data: + # Base64 encoded Dynatrace API token + # Replace with your actual token: echo -n "your-api-token" | base64 + api-token: eW91ci1hcGktdG9rZW4= \ No newline at end of file diff --git a/examples/datasink/metric-using-dynatrace-prod.yaml b/examples/datasink/metric-using-dynatrace-prod.yaml new file mode 100644 index 0000000..8ec100d --- /dev/null +++ b/examples/datasink/metric-using-dynatrace-prod.yaml @@ -0,0 +1,14 @@ +# Metric Custom Resource using Dynatrace Production DataSink +apiVersion: metrics.cloud.sap/v1alpha1 +kind: Metric +metadata: + name: example-metric-dynatrace-prod + namespace: metrics-operator-system +spec: + dataSinkRef: + name: default + interval: "10m" + target: + kind: ConfigMap + group: "" + version: v1 diff --git a/examples/dev-core/metric.yaml b/examples/dev-core/metric.yaml index f03eed8..6ded561 100644 --- a/examples/dev-core/metric.yaml +++ b/examples/dev-core/metric.yaml @@ -10,4 +10,8 @@ spec: group: landscaper-service.gardener.cloud version: v1alpha1 interval: "1m" # in minutes + # Uses default DataSink (named "default" in metrics-operator-system namespace) + # To use a custom DataSink, uncomment and specify: + # dataSinkRef: + # name: custom-sink --- diff --git a/examples/managed_metric.yaml b/examples/managed_metric.yaml index 26fb598..cc8455e 100644 --- a/examples/managed_metric.yaml +++ b/examples/managed_metric.yaml @@ -9,3 +9,7 @@ spec: group: helm.crossplane.io version: v1beta1 interval: "1m" # in minutes + # Uses default DataSink (named "default" in metrics-operator-system namespace) + # To use a custom DataSink, uncomment and specify: + dataSinkRef: + name: other-datasink diff --git a/examples/remoteclusteraccess/metric_with_rca.yaml b/examples/remoteclusteraccess/metric_with_rca.yaml index 19952d9..11e568e 100644 --- a/examples/remoteclusteraccess/metric_with_rca.yaml +++ b/examples/remoteclusteraccess/metric_with_rca.yaml @@ -13,6 +13,10 @@ spec: remoteClusterAccessRef: name: crate-cluster namespace: test-monitoring + # Uses default DataSink (named "default" in metrics-operator-system namespace) + # To use a custom DataSink, uncomment and specify: + # dataSinkRef: + # name: custom-sink --- apiVersion: metrics.cloud.sap/v1alpha1 kind: Metric @@ -29,6 +33,10 @@ spec: remoteClusterAccessRef: name: crate-cluster namespace: test-monitoring + # Uses default DataSink (named "default" in metrics-operator-system namespace) + # To use a custom DataSink, uncomment and specify: + # dataSinkRef: + # name: custom-sink --- apiVersion: metrics.cloud.sap/v1alpha1 kind: Metric @@ -45,4 +53,8 @@ spec: remoteClusterAccessRef: name: crate-cluster namespace: test-monitoring + # Uses default DataSink (named "default" in metrics-operator-system namespace) + # To use a custom DataSink, uncomment and specify: + # dataSinkRef: + # name: custom-sink --- diff --git a/examples/v1beta1/fedmetric.yaml b/examples/v1beta1/fedmetric.yaml index a6d220d..9a30267 100644 --- a/examples/v1beta1/fedmetric.yaml +++ b/examples/v1beta1/fedmetric.yaml @@ -16,4 +16,8 @@ spec: federateClusterAccessRef: name: federate-ca-sample namespace: default + # Uses default DataSink (named "default" in metrics-operator-system namespace) + # To use a custom DataSink, uncomment and specify: + # dataSinkRef: + # name: custom-sink --- diff --git a/internal/clientoptl/optel_test.go b/internal/clientoptl/optel_test.go index dd46e91..26684ef 100644 --- a/internal/clientoptl/optel_test.go +++ b/internal/clientoptl/optel_test.go @@ -7,7 +7,7 @@ import ( func TestNewMetricClient_Real(t *testing.T) { t.Skip("skipping test") - client, err := NewMetricClient(context.TODO(), "canary.eu21.apm.services.cloud.sap", "/e/1b9c6fb0-eb17-4fce-96b0-088cee0861b3/api/v2/", "dt0c01..") + client, err := NewMetricClient(context.TODO(), "https://canary.eu21.apm.services.cloud.sap/e/1b9c6fb0-eb17-4fce-96b0-088cee0861b3/api/v2/otlp/v1/metrics", "dt0c01..") if err != nil { t.Errorf("Failed to create OTLP exporter: %v", err) diff --git a/internal/clientoptl/optl.go b/internal/clientoptl/optl.go index d14aca7..24f02a9 100644 --- a/internal/clientoptl/optl.go +++ b/internal/clientoptl/optl.go @@ -3,7 +3,7 @@ package clientoptl import ( "context" "fmt" - "path" + "net/url" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -53,22 +53,34 @@ func (dp *DataPoint) SetValue(value int64) *DataPoint { } // NewMetricClient creates a new metric client -func NewMetricClient(ctx context.Context, dtAPIHost, dtAPIBasePath, dtAPIToken string) (*MetricClient, error) { +func NewMetricClient(ctx context.Context, dtAPIHost, dtAPIToken string) (*MetricClient, error) { authHeader := map[string]string{"Authorization": "Api-Token " + dtAPIToken} deltaTemporalitySelector := func(sdkmetric.InstrumentKind) metricdata.Temporality { return metricdata.DeltaTemporality } - urlPath := path.Join(dtAPIBasePath, "/otlp/v1/metrics") + // Parse the dtAPIHost URL to extract host and path components + // dtAPIHost is the full endpoint from DataSink, e.g., "https://.../otlp/v1/metrics" + parsedURL, err := url.Parse(dtAPIHost) + if err != nil { + return nil, fmt.Errorf("failed to parse endpoint URL: %w", err) + } - metricsExporter, err := otlpmetrichttp.New( - ctx, - otlpmetrichttp.WithEndpoint(dtAPIHost), - otlpmetrichttp.WithURLPath(urlPath), + // Construct OTLP options with proper URL parsing + opts := []otlpmetrichttp.Option{ + otlpmetrichttp.WithEndpoint(parsedURL.Host), + otlpmetrichttp.WithURLPath(parsedURL.Path), // Use the path directly from the DataSink endpoint otlpmetrichttp.WithHeaders(authHeader), otlpmetrichttp.WithTemporalitySelector(deltaTemporalitySelector), - ) + } + + // Add insecure option if scheme is http + if parsedURL.Scheme == "http" { + opts = append(opts, otlpmetrichttp.WithInsecure()) + } + + metricsExporter, err := otlpmetrichttp.New(ctx, opts...) if err != nil { return nil, fmt.Errorf("failed to create OTLP exporter: %w", err) } diff --git a/internal/common/common.go b/internal/common/common.go index 6d7473c..65cac8f 100644 --- a/internal/common/common.go +++ b/internal/common/common.go @@ -1,49 +1,5 @@ package common -import ( - "context" - - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - - corev1 "k8s.io/api/core/v1" -) - -const ( - // SecretNameSpace is the namespace where the secret is deployed - SecretNameSpace string = "co-metrics-operator" - // SecretName is the name of the secret - SecretName string = "co-dynatrace-credentials" -) - -// GetCredentialsSecret Get the Secret with access token from the cluster, which you deployed earlier into the system -// -// Deployment of secret: -// -// you can deploy the metric through: kubectl apply -f sample/secret.yaml -func GetCredentialsSecret(ctx context.Context, client client.Client) (*corev1.Secret, error) { - secret := &corev1.Secret{} - namespace := types.NamespacedName{ - Namespace: SecretNameSpace, - Name: SecretName, - } - err := client.Get(ctx, namespace, secret) - if err != nil { - return &corev1.Secret{}, err - } - return secret, nil -} - -// GetCredentialData returns the data from the secret -func GetCredentialData(secret *corev1.Secret) DataSinkCredentials { - creds := DataSinkCredentials{ - Host: string(secret.Data["Host"]), - Path: string(secret.Data["Path"]), - Token: string(secret.Data["Token"]), - } - return creds -} - // DataSinkCredentials holds the credentials to access the data sink (e.g. dynatrace) type DataSinkCredentials struct { Host string diff --git a/internal/controller/datasink_utils.go b/internal/controller/datasink_utils.go new file mode 100644 index 0000000..50b8f8c --- /dev/null +++ b/internal/controller/datasink_utils.go @@ -0,0 +1,142 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "fmt" + "os" + + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/SAP/metrics-operator/api/v1alpha1" + "github.com/SAP/metrics-operator/internal/common" +) + +// DataSinkCredentialsRetriever provides common functionality for retrieving DataSink credentials +type DataSinkCredentialsRetriever struct { + client client.Client + recorder record.EventRecorder +} + +// NewDataSinkCredentialsRetriever creates a new DataSinkCredentialsRetriever +func NewDataSinkCredentialsRetriever(client client.Client, recorder record.EventRecorder) *DataSinkCredentialsRetriever { + return &DataSinkCredentialsRetriever{ + client: client, + recorder: recorder, + } +} + +// GetDataSinkCredentials fetches DataSink configuration and credentials for any metric type +func (d *DataSinkCredentialsRetriever) GetDataSinkCredentials(ctx context.Context, dataSinkRef *v1alpha1.DataSinkReference, eventObject client.Object, l logr.Logger) (common.DataSinkCredentials, error) { + // Determine the namespace where DataSinks are expected to be found. + dataSinkLookupNamespace := os.Getenv("OPERATOR_CONFIG_NAMESPACE") + if dataSinkLookupNamespace == "" { + l.V(1).Info("OPERATOR_CONFIG_NAMESPACE not set, trying POD_NAMESPACE.") + dataSinkLookupNamespace = os.Getenv("POD_NAMESPACE") + if dataSinkLookupNamespace == "" { + l.Info("Neither OPERATOR_CONFIG_NAMESPACE nor POD_NAMESPACE is set. Defaulting DataSink lookup to 'default' namespace.") + dataSinkLookupNamespace = "default" + } else { + l.Info("Using POD_NAMESPACE for DataSink lookup.", "namespace", dataSinkLookupNamespace) + } + } else { + l.Info("Using OPERATOR_CONFIG_NAMESPACE for DataSink lookup.", "namespace", dataSinkLookupNamespace) + } + + // Determine DataSink name + dataSinkName := "default" + if dataSinkRef != nil && dataSinkRef.Name != "" { + dataSinkName = dataSinkRef.Name + } + + // Fetch DataSink CR + dataSink := &v1alpha1.DataSink{} + dataSinkKey := types.NamespacedName{ + Namespace: dataSinkLookupNamespace, + Name: dataSinkName, + } + + if err := d.client.Get(ctx, dataSinkKey, dataSink); err != nil { + if apierrors.IsNotFound(err) { + l.Error(err, fmt.Sprintf("DataSink '%s' not found in namespace '%s'", dataSinkName, dataSinkLookupNamespace)) + d.recorder.Event(eventObject, "Error", "DataSinkNotFound", fmt.Sprintf("DataSink '%s' not found in namespace '%s'", dataSinkName, dataSinkLookupNamespace)) + } else { + l.Error(err, fmt.Sprintf("unable to fetch DataSink '%s' in namespace '%s'", dataSinkName, dataSinkLookupNamespace)) + d.recorder.Event(eventObject, "Error", "DataSinkFetchError", fmt.Sprintf("unable to fetch DataSink '%s' in namespace '%s'", dataSinkName, dataSinkLookupNamespace)) + } + return common.DataSinkCredentials{}, err + } + + // Extract endpoint and protocol from DataSink + endpoint := dataSink.Spec.Connection.Endpoint + protocol := dataSink.Spec.Connection.Protocol + + // Handle authentication + var token string + if dataSink.Spec.Authentication != nil && dataSink.Spec.Authentication.APIKey != nil { + // Fetch credentials secret + secretName := dataSink.Spec.Authentication.APIKey.SecretKeyRef.Name + secretKey := dataSink.Spec.Authentication.APIKey.SecretKeyRef.Key + + secret := &corev1.Secret{} + secretNamespacedName := types.NamespacedName{ + Namespace: dataSinkLookupNamespace, + Name: secretName, + } + + if err := d.client.Get(ctx, secretNamespacedName, secret); err != nil { + if apierrors.IsNotFound(err) { + l.Error(err, fmt.Sprintf("Secret '%s' not found in namespace '%s'", secretName, dataSinkLookupNamespace)) + d.recorder.Event(eventObject, "Error", "SecretNotFound", fmt.Sprintf("Secret '%s' not found in namespace '%s'", secretName, dataSinkLookupNamespace)) + } else { + l.Error(err, fmt.Sprintf("unable to fetch Secret '%s' in namespace '%s'", secretName, dataSinkLookupNamespace)) + d.recorder.Event(eventObject, "Error", "SecretFetchError", fmt.Sprintf("unable to fetch Secret '%s' in namespace '%s'", secretName, dataSinkLookupNamespace)) + } + return common.DataSinkCredentials{}, err + } + + // Extract token from secret + tokenBytes, exists := secret.Data[secretKey] + if !exists { + err := fmt.Errorf("key '%s' not found in secret '%s'", secretKey, secretName) + l.Error(err, fmt.Sprintf("key '%s' not found in secret '%s'", secretKey, secretName)) + d.recorder.Event(eventObject, "Error", "SecretKeyNotFound", fmt.Sprintf("key '%s' not found in secret '%s'", secretKey, secretName)) + return common.DataSinkCredentials{}, err + } + token = string(tokenBytes) + } + + // Construct credentials compatible with clientoptl.NewMetricClient + // The NewMetricClient expects: dtAPIHost, dtAPIBasePath, dtAPIToken + // For now, we'll use the full endpoint as Host and empty Path + // TODO: Parse endpoint to separate host and path if needed based on protocol + credentials := common.DataSinkCredentials{ + Host: endpoint, // Full endpoint URL (e.g., https://example.dynatrace.com) + Path: "", // Base path for API (will be combined with /otlp/v1/metrics in clientoptl) + Token: token, + } + + l.Info(fmt.Sprintf("Using DataSink '%s' with protocol '%s' and endpoint '%s'", dataSinkName, protocol, endpoint)) + + return credentials, nil +} diff --git a/internal/controller/federatedmanagedmetric_controller.go b/internal/controller/federatedmanagedmetric_controller.go index dd018f6..b16dbd3 100644 --- a/internal/controller/federatedmanagedmetric_controller.go +++ b/internal/controller/federatedmanagedmetric_controller.go @@ -67,6 +67,12 @@ func (r *FederatedManagedMetricReconciler) getRestConfig() *rest.Config { return r.RestConfig } +// getDataSinkCredentials fetches DataSink configuration and credentials +func (r *FederatedManagedMetricReconciler) getDataSinkCredentials(ctx context.Context, federatedManagedMetric *v1alpha1.FederatedManagedMetric, l logr.Logger) (common.DataSinkCredentials, error) { + retriever := NewDataSinkCredentialsRetriever(r.getClient(), r.Recorder) + return retriever.GetDataSinkCredentials(ctx, federatedManagedMetric.Spec.DataSinkRef, federatedManagedMetric, l) +} + func (r *FederatedManagedMetricReconciler) handleGetError(err error, log logr.Logger) (ctrl.Result, error) { // We'll ignore not-found errors. They can't be fixed by an immediate requeue. // We'll need to wait for a new notification. We can also get them on delete requests. @@ -99,6 +105,8 @@ func (r *FederatedManagedMetricReconciler) shouldReconcile(metric *v1alpha1.Fede // +kubebuilder:rbac:groups=metrics.cloud.sap,resources=federatedmanagedmetrics,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=metrics.cloud.sap,resources=federatedmanagedmetrics/status,verbs=get;update;patch // +kubebuilder:rbac:groups=metrics.cloud.sap,resources=federatedmanagedmetrics/finalizers,verbs=update +// +kubebuilder:rbac:groups=metrics.cloud.sap,resources=datasinks,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=secrets,verbs=get // //nolint:gocyclo func (r *FederatedManagedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ -123,15 +131,13 @@ func (r *FederatedManagedMetricReconciler) Reconcile(ctx context.Context, req ct } /* - 1.1 Get the Secret that holds the Dynatrace credentials + 1.1 Get the DataSink credentials */ - secret, errSecret := common.GetCredentialsSecret(ctx, r.getClient()) - if errSecret != nil { - return r.handleSecretError(l, errSecret, metric) + credentials, errCredentials := r.getDataSinkCredentials(ctx, &metric, l) + if errCredentials != nil { + return ctrl.Result{RequeueAfter: RequeueAfterError}, errCredentials } - credentials := common.GetCredentialData(secret) - /* 1.2 Create QueryConfig to query the resources in the K8S cluster or external cluster based on the kubeconfig secret reference */ @@ -141,7 +147,7 @@ func (r *FederatedManagedMetricReconciler) Reconcile(ctx context.Context, req ct return ctrl.Result{RequeueAfter: RequeueAfterError}, err } - metricClient, errCli := clientoptl.NewMetricClient(ctx, credentials.Host, credentials.Path, credentials.Token) + metricClient, errCli := clientoptl.NewMetricClient(ctx, credentials.Host, credentials.Token) if errCli != nil { l.Error(errCli, fmt.Sprintf("federated managed metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) @@ -218,12 +224,6 @@ func (r *FederatedManagedMetricReconciler) Reconcile(ctx context.Context, req ct }, nil } -func (r *FederatedManagedMetricReconciler) handleSecretError(l logr.Logger, errSecret error, metric v1alpha1.FederatedManagedMetric) (ctrl.Result, error) { - l.Error(errSecret, fmt.Sprintf("unable to fetch secret '%s' in namespace '%s' that stores the credentials to data sink", common.SecretName, common.SecretNameSpace)) - r.Recorder.Event(&metric, "Error", "SecretNotFound", fmt.Sprintf("unable to fetch secret '%s' in namespace '%s' that stores the credentials to data sink", common.SecretName, common.SecretNameSpace)) - return ctrl.Result{RequeueAfter: RequeueAfterError}, errSecret -} - // SetupWithManager sets up the controller with the Manager. func (r *FederatedManagedMetricReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/internal/controller/federatedmetric_controller.go b/internal/controller/federatedmetric_controller.go index f612c8d..f2e4402 100644 --- a/internal/controller/federatedmetric_controller.go +++ b/internal/controller/federatedmetric_controller.go @@ -18,26 +18,23 @@ package controller import ( "context" - "fmt" - "time" "github.com/go-logr/logr" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/SAP/metrics-operator/api/v1alpha1" "github.com/SAP/metrics-operator/internal/clientoptl" "github.com/SAP/metrics-operator/internal/common" "github.com/SAP/metrics-operator/internal/config" orc "github.com/SAP/metrics-operator/internal/orchestrator" - - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/rest" - "k8s.io/client-go/tools/record" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" ) // NewFederatedMetricReconciler creates a new FederatedMetricReconciler @@ -70,6 +67,12 @@ func (r *FederatedMetricReconciler) getRestConfig() *rest.Config { return r.RestConfig } +// getDataSinkCredentials fetches DataSink configuration and credentials +func (r *FederatedMetricReconciler) getDataSinkCredentials(ctx context.Context, federatedMetric *v1alpha1.FederatedMetric, l logr.Logger) (common.DataSinkCredentials, error) { + retriever := NewDataSinkCredentialsRetriever(r.getClient(), r.Recorder) + return retriever.GetDataSinkCredentials(ctx, federatedMetric.Spec.DataSinkRef, federatedMetric, l) +} + func handleGetError(err error, log logr.Logger) (ctrl.Result, error) { // we'll ignore not-found errors, since they can't be fixed by an immediate // requeue (we'll need to wait for a new notification), and we can also get them @@ -102,6 +105,8 @@ func shouldReconcile(metric *v1alpha1.FederatedMetric) bool { // +kubebuilder:rbac:groups=metrics.cloud.sap,resources=federatedmetrics,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=metrics.cloud.sap,resources=federatedmetrics/status,verbs=get;update;patch // +kubebuilder:rbac:groups=metrics.cloud.sap,resources=federatedmetrics/finalizers,verbs=update +// +kubebuilder:rbac:groups=metrics.cloud.sap,resources=datasinks,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=secrets,verbs=get // Reconcile handles the reconciliation of the FederatedMetric object // @@ -128,15 +133,13 @@ func (r *FederatedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Requ } /* - 1.1 Get the Secret that holds the Dynatrace credentials + 1.1 Get DataSink configuration and credentials */ - secret, errSecret := common.GetCredentialsSecret(ctx, r.getClient()) - if errSecret != nil { - return r.handleSecretError(l, errSecret, metric) + credentials, err := r.getDataSinkCredentials(ctx, &metric, l) + if err != nil { + return ctrl.Result{RequeueAfter: RequeueAfterError}, err } - credentials := common.GetCredentialData(secret) - /* 1.2 Create QueryConfig to query the resources in the K8S cluster or external cluster based on the kubeconfig secret reference */ @@ -146,7 +149,7 @@ func (r *FederatedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{RequeueAfter: RequeueAfterError}, err } - metricClient, errCli := clientoptl.NewMetricClient(ctx, credentials.Host, credentials.Path, credentials.Token) + metricClient, errCli := clientoptl.NewMetricClient(ctx, credentials.Host, credentials.Token) if errCli != nil { l.Error(errCli, fmt.Sprintf("federated metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) @@ -223,12 +226,6 @@ func (r *FederatedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Requ }, nil } -func (r *FederatedMetricReconciler) handleSecretError(l logr.Logger, errSecret error, metric v1alpha1.FederatedMetric) (ctrl.Result, error) { - l.Error(errSecret, fmt.Sprintf("unable to fetch secret '%s' in namespace '%s' that stores the credentials to data sink", common.SecretName, common.SecretNameSpace)) - r.Recorder.Event(&metric, "Error", "SecretNotFound", fmt.Sprintf("unable to fetch secret '%s' in namespace '%s' that stores the credentials to data sink", common.SecretName, common.SecretNameSpace)) - return ctrl.Result{RequeueAfter: RequeueAfterError}, errSecret -} - // SetupWithManager sets up the controller with the Manager. func (r *FederatedMetricReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/internal/controller/managedmetric_controller.go b/internal/controller/managedmetric_controller.go index 37394ea..c90a12b 100644 --- a/internal/controller/managedmetric_controller.go +++ b/internal/controller/managedmetric_controller.go @@ -35,6 +35,7 @@ import ( "github.com/SAP/metrics-operator/internal/common" "github.com/SAP/metrics-operator/internal/config" "github.com/SAP/metrics-operator/internal/orchestrator" + "github.com/go-logr/logr" ctrl "sigs.k8s.io/controller-runtime" @@ -84,9 +85,17 @@ type ManagedMetricReconciler struct { Recorder record.EventRecorder } +// getDataSinkCredentials fetches DataSink configuration and credentials +func (r *ManagedMetricReconciler) getDataSinkCredentials(ctx context.Context, managedMetric *v1alpha1.ManagedMetric, l logr.Logger) (common.DataSinkCredentials, error) { + retriever := NewDataSinkCredentialsRetriever(r.getClient(), r.Recorder) + return retriever.GetDataSinkCredentials(ctx, managedMetric.Spec.DataSinkRef, managedMetric, l) +} + //+kubebuilder:rbac:groups=metrics.cloud.sap,resources=managedmetrics,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=metrics.cloud.sap,resources=managedmetrics/status,verbs=get;update;patch //+kubebuilder:rbac:groups=metrics.cloud.sap,resources=managedmetrics/finalizers,verbs=update +//+kubebuilder:rbac:groups=metrics.cloud.sap,resources=datasinks,verbs=get;list;watch +//+kubebuilder:rbac:groups="",resources=secrets,verbs=get // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. @@ -120,17 +129,13 @@ func (r *ManagedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Reques } /* - 1.1 Get the Secret that holds the Dynatrace credentials + 1.1 Get DataSink configuration and credentials */ - secret, errSecret := common.GetCredentialsSecret(ctx, r.inClient) - if errSecret != nil { - l.Error(errSecret, fmt.Sprintf("unable to fetch Secret '%s' in namespace '%s' that stores the credentials to Data Sink", common.SecretName, common.SecretNameSpace)) - r.Recorder.Event(&metric, "Error", "SecretNotFound", fmt.Sprintf("unable to fetch Secret '%s' in namespace '%s' that stores the credentials to Data Sink", common.SecretName, common.SecretNameSpace)) - return ctrl.Result{RequeueAfter: RequeueAfterError}, errSecret + credentials, err := r.getDataSinkCredentials(ctx, &metric, l) + if err != nil { + return ctrl.Result{RequeueAfter: RequeueAfterError}, err } - credentials := common.GetCredentialData(secret) - /* 1.2 Create QueryConfig to query the resources in the K8S cluster or external cluster based on the kubeconfig secret reference */ @@ -142,7 +147,7 @@ func (r *ManagedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Reques /* 1.3 Create OTel metric client and gauge metric */ - metricClient, errCli := clientoptl.NewMetricClient(ctx, credentials.Host, credentials.Path, credentials.Token) + metricClient, errCli := clientoptl.NewMetricClient(ctx, credentials.Host, credentials.Token) if errCli != nil { l.Error(errCli, fmt.Sprintf("managed metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) return ctrl.Result{RequeueAfter: RequeueAfterError}, errCli diff --git a/internal/controller/metric_controller.go b/internal/controller/metric_controller.go index 66a68d4..9ce0fdc 100644 --- a/internal/controller/metric_controller.go +++ b/internal/controller/metric_controller.go @@ -74,6 +74,12 @@ func (r *MetricReconciler) getRestConfig() *rest.Config { return r.RestConfig } +// getDataSinkCredentials fetches DataSink configuration and credentials +func (r *MetricReconciler) getDataSinkCredentials(ctx context.Context, metric *v1alpha1.Metric, l logr.Logger) (common.DataSinkCredentials, error) { + retriever := NewDataSinkCredentialsRetriever(r.getClient(), r.Recorder) + return retriever.GetDataSinkCredentials(ctx, metric.Spec.DataSinkRef, metric, l) +} + func (r *MetricReconciler) scheduleNextReconciliation(metric *v1alpha1.Metric) (ctrl.Result, error) { elapsed := time.Since(metric.Status.Observation.Timestamp.Time) @@ -106,6 +112,8 @@ func (r *MetricReconciler) handleGetError(err error, log logr.Logger) (ctrl.Resu // +kubebuilder:rbac:groups=metrics.cloud.sap,resources=metrics,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=metrics.cloud.sap,resources=metrics/status,verbs=get;update;patch // +kubebuilder:rbac:groups=metrics.cloud.sap,resources=metrics/finalizers,verbs=update +// +kubebuilder:rbac:groups=metrics.cloud.sap,resources=datasinks,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=secrets,verbs=get // Reconcile handles the reconciliation of a Metric object // @@ -130,17 +138,13 @@ func (r *MetricReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } /* - 1.1 Get the Secret that holds the Dynatrace credentials + 1.1 Get DataSink configuration and credentials */ - secret, errSecret := common.GetCredentialsSecret(ctx, r.getClient()) - if errSecret != nil { - l.Error(errSecret, fmt.Sprintf("unable to fetch secret '%s' in namespace '%s' that stores the credentials to data sink", common.SecretName, common.SecretNameSpace)) - r.Recorder.Event(&metric, "Error", "SecretNotFound", fmt.Sprintf("unable to fetch secret '%s' in namespace '%s' that stores the credentials to data sink", common.SecretName, common.SecretNameSpace)) - return ctrl.Result{RequeueAfter: RequeueAfterError}, errSecret + credentials, err := r.getDataSinkCredentials(ctx, &metric, l) + if err != nil { + return ctrl.Result{RequeueAfter: RequeueAfterError}, err } - credentials := common.GetCredentialData(secret) - /* 1.2 Create QueryConfig to query the resources in the K8S cluster or external cluster based on the kubeconfig secret reference */ @@ -149,7 +153,7 @@ func (r *MetricReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{RequeueAfter: RequeueAfterError}, err } - metricClient, errCli := clientoptl.NewMetricClient(ctx, credentials.Host, credentials.Path, credentials.Token) + metricClient, errCli := clientoptl.NewMetricClient(ctx, credentials.Host, credentials.Token) if errCli != nil { l.Error(errCli, fmt.Sprintf("metric '%s' failed to create OTel client, re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) // TODO: Update status? diff --git a/internal/controller/metric_controller_test.go b/internal/controller/metric_controller_test.go index e5c7830..25d4743 100644 --- a/internal/controller/metric_controller_test.go +++ b/internal/controller/metric_controller_test.go @@ -27,7 +27,6 @@ import ( "github.com/SAP/metrics-operator/internal/common" orc "github.com/SAP/metrics-operator/internal/orchestrator" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" @@ -166,7 +165,7 @@ func TestMetricController(t *testing.T) { // Run the tests t.Run("TestReconcileMetricHappyPath", testReconcileMetricHappyPath) t.Run("TestReconcileMetricNotFound", testReconcileMetricNotFound) - t.Run("TestReconcileSecretNotFound", testReconcileSecretNotFound) + t.Run("TestReconcileDataSinkNotFound", testReconcileSecretNotFound) } // testReconcileMetricNotFound tests the behavior when the Metric is not found @@ -203,23 +202,23 @@ func testReconcileMetricNotFound(t *testing.T) { require.Equal(t, RequeueAfterError, result.RequeueAfter, "Should requeue after error time") } -// testReconcileSecretNotFound tests the behavior when the Secret is not found +// testReconcileSecretNotFound tests the behavior when the DataSink is not found func testReconcileSecretNotFound(t *testing.T) { const ( - MetricName = "test-metric-no-secret" + MetricName = "test-metric-no-datasink" MetricNamespace = "default" ) ctx := context.Background() - // Create a test Metric + // Create a test Metric (without DataSinkRef, so it will look for "default" DataSink) metric := &v1alpha1.Metric{ ObjectMeta: metav1.ObjectMeta{ Name: MetricName, Namespace: MetricNamespace, }, Spec: v1alpha1.MetricSpec{ - Name: "test-metric-no-secret", + Name: "test-metric-no-datasink", Description: "Test metric description", Target: v1alpha1.GroupVersionKind{ Kind: "Pod", @@ -259,48 +258,22 @@ func testReconcileSecretNotFound(t *testing.T) { result, err := reconciler.Reconcile(ctx, req) // Verify the result - require.Error(t, err, "Reconcile should return an error when Secret is not found") + require.Error(t, err, "Reconcile should return an error when DataSink is not found") require.Equal(t, RequeueAfterError, result.RequeueAfter, "Should requeue after error time") - // Verify that events were recorded + // Verify that events were recorded - now expecting DataSinkNotFound instead of SecretNotFound event := <-recorder.Events - require.Contains(t, event, "SecretNotFound") + require.Contains(t, event, "DataSinkNotFound") } func testReconcileMetricHappyPath(t *testing.T) { const ( MetricName = "test-metric" MetricNamespace = "default" - SecretName = common.SecretName - SecretNamespace = common.SecretNameSpace ) ctx := context.Background() - // Create namespace for the secret - namespace := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: SecretNamespace, - }, - } - err := k8sClient.Create(ctx, namespace) - require.NoError(t, err) - - // Create the Secret with credentials - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: SecretName, - Namespace: SecretNamespace, - }, - Data: map[string][]byte{ - "Host": []byte("test-host"), - "Path": []byte("test-path"), - "Token": []byte("test-token"), - }, - } - err = k8sClient.Create(ctx, secret) - require.NoError(t, err) - // Create a test Metric metric := &v1alpha1.Metric{ ObjectMeta: metav1.ObjectMeta{ @@ -318,7 +291,7 @@ func testReconcileMetricHappyPath(t *testing.T) { Interval: metav1.Duration{Duration: 5 * time.Minute}, }, } - err = k8sClient.Create(ctx, metric) + err := k8sClient.Create(ctx, metric) require.NoError(t, err) // Set up fake implementations @@ -354,13 +327,7 @@ func testReconcileMetricHappyPath(t *testing.T) { // Clean up resources after test defer func() { - err := k8sClient.Delete(ctx, secret) - require.NoError(t, err) - - err = k8sClient.Delete(ctx, metric) - require.NoError(t, err) - - err = k8sClient.Delete(ctx, namespace) + err := k8sClient.Delete(ctx, metric) require.NoError(t, err) }() From 9dfb1a66f1a6519adf2482aad48135587a710c62 Mon Sep 17 00:00:00 2001 From: Mirza Kopic Date: Mon, 2 Jun 2025 22:22:39 +0200 Subject: [PATCH 2/5] simplify data sink connection for now --- api/v1alpha1/datasink_types.go | 7 ------- cmd/embedded/crds/metrics.cloud.sap_datasinks.yaml | 14 -------------- config/crd/bases/metrics.cloud.sap_datasinks.yaml | 14 -------------- examples/datasink/basic-datasink.yaml | 2 -- internal/controller/datasink_utils.go | 5 ++--- 5 files changed, 2 insertions(+), 40 deletions(-) diff --git a/api/v1alpha1/datasink_types.go b/api/v1alpha1/datasink_types.go index d129ff9..b19b3fb 100644 --- a/api/v1alpha1/datasink_types.go +++ b/api/v1alpha1/datasink_types.go @@ -25,12 +25,6 @@ import ( type Connection struct { // Endpoint specifies the target endpoint URL Endpoint string `json:"endpoint"` - // Protocol specifies the communication protocol - // +kubebuilder:validation:Enum=http;grpc - Protocol string `json:"protocol"` - // InsecureSkipVerify controls whether to skip TLS certificate verification - // +optional - InsecureSkipVerify bool `json:"insecureSkipVerify,omitempty"` } // APIKeyAuthentication defines API key authentication configuration @@ -66,7 +60,6 @@ type DataSinkStatus struct { // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:printcolumn:name="ENDPOINT",type="string",JSONPath=".spec.connection.endpoint" -// +kubebuilder:printcolumn:name="PROTOCOL",type="string",JSONPath=".spec.connection.protocol" // +kubebuilder:printcolumn:name="AGE",type="date",JSONPath=".metadata.creationTimestamp" type DataSink struct { metav1.TypeMeta `json:",inline"` diff --git a/cmd/embedded/crds/metrics.cloud.sap_datasinks.yaml b/cmd/embedded/crds/metrics.cloud.sap_datasinks.yaml index 96a5946..91f92b7 100644 --- a/cmd/embedded/crds/metrics.cloud.sap_datasinks.yaml +++ b/cmd/embedded/crds/metrics.cloud.sap_datasinks.yaml @@ -18,9 +18,6 @@ spec: - jsonPath: .spec.connection.endpoint name: ENDPOINT type: string - - jsonPath: .spec.connection.protocol - name: PROTOCOL - type: string - jsonPath: .metadata.creationTimestamp name: AGE type: date @@ -91,19 +88,8 @@ spec: endpoint: description: Endpoint specifies the target endpoint URL type: string - insecureSkipVerify: - description: InsecureSkipVerify controls whether to skip TLS certificate - verification - type: boolean - protocol: - description: Protocol specifies the communication protocol - enum: - - http - - grpc - type: string required: - endpoint - - protocol type: object required: - connection diff --git a/config/crd/bases/metrics.cloud.sap_datasinks.yaml b/config/crd/bases/metrics.cloud.sap_datasinks.yaml index 96a5946..91f92b7 100644 --- a/config/crd/bases/metrics.cloud.sap_datasinks.yaml +++ b/config/crd/bases/metrics.cloud.sap_datasinks.yaml @@ -18,9 +18,6 @@ spec: - jsonPath: .spec.connection.endpoint name: ENDPOINT type: string - - jsonPath: .spec.connection.protocol - name: PROTOCOL - type: string - jsonPath: .metadata.creationTimestamp name: AGE type: date @@ -91,19 +88,8 @@ spec: endpoint: description: Endpoint specifies the target endpoint URL type: string - insecureSkipVerify: - description: InsecureSkipVerify controls whether to skip TLS certificate - verification - type: boolean - protocol: - description: Protocol specifies the communication protocol - enum: - - http - - grpc - type: string required: - endpoint - - protocol type: object required: - connection diff --git a/examples/datasink/basic-datasink.yaml b/examples/datasink/basic-datasink.yaml index e207f21..a71f642 100644 --- a/examples/datasink/basic-datasink.yaml +++ b/examples/datasink/basic-datasink.yaml @@ -7,8 +7,6 @@ metadata: spec: connection: endpoint: "https://your-tenant.live.dynatrace.com/api/v2/metrics/ingest/otlp/v1/metrics" - protocol: "http" - insecureSkipVerify: false authentication: apiKey: secretKeyRef: diff --git a/internal/controller/datasink_utils.go b/internal/controller/datasink_utils.go index 50b8f8c..0cc4095 100644 --- a/internal/controller/datasink_utils.go +++ b/internal/controller/datasink_utils.go @@ -87,9 +87,8 @@ func (d *DataSinkCredentialsRetriever) GetDataSinkCredentials(ctx context.Contex return common.DataSinkCredentials{}, err } - // Extract endpoint and protocol from DataSink + // Extract endpoint from DataSink endpoint := dataSink.Spec.Connection.Endpoint - protocol := dataSink.Spec.Connection.Protocol // Handle authentication var token string @@ -136,7 +135,7 @@ func (d *DataSinkCredentialsRetriever) GetDataSinkCredentials(ctx context.Contex Token: token, } - l.Info(fmt.Sprintf("Using DataSink '%s' with protocol '%s' and endpoint '%s'", dataSinkName, protocol, endpoint)) + l.Info(fmt.Sprintf("Using DataSink '%s' with endpoint '%s'", dataSinkName, endpoint)) return credentials, nil } From 6d52d46b80da423fdc5407efb75cd8997c852b7a Mon Sep 17 00:00:00 2001 From: Mirza Kopic Date: Mon, 2 Jun 2025 23:04:18 +0200 Subject: [PATCH 3/5] update status correctly, ensure it is always set --- api/v1alpha1/conditions.go | 10 ++++ internal/common/conditions.go | 33 ++++++++++++ .../federatedmanagedmetric_controller.go | 38 ++++++++++--- .../controller/federatedmetric_controller.go | 38 ++++++++++--- .../controller/managedmetric_controller.go | 47 +++++++++++----- internal/controller/metric_controller.go | 54 ++++++++++++------- 6 files changed, 170 insertions(+), 50 deletions(-) diff --git a/api/v1alpha1/conditions.go b/api/v1alpha1/conditions.go index c22165f..0f03d06 100644 --- a/api/v1alpha1/conditions.go +++ b/api/v1alpha1/conditions.go @@ -33,4 +33,14 @@ const ( // TypeError is a generic condition type that indicates an error has occurred TypeError = "Error" + + // TypeReady is a condition type that indicates the resource is ready + TypeReady = "Ready" + + // StatusStringTrue represents the True status string. + StatusStringTrue string = "True" + // StatusStringFalse represents the False status string. + StatusStringFalse string = "False" + // StatusStringUnknown represents the Unknown status string. + StatusStringUnknown string = "Unknown" ) diff --git a/internal/common/conditions.go b/internal/common/conditions.go index e36d08b..16b9d77 100644 --- a/internal/common/conditions.go +++ b/internal/common/conditions.go @@ -37,6 +37,39 @@ func Updated() metav1.Condition { } } +// ReadyTrue returns a condition that indicates the resource is ready and functioning correctly +func ReadyTrue(message string) metav1.Condition { + return metav1.Condition{ + Type: v1alpha1.TypeReady, + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "ReconciliationSucceeded", + Message: message, + } +} + +// ReadyFalse returns a condition that indicates the resource is not ready due to an error +func ReadyFalse(reason, message string) metav1.Condition { + return metav1.Condition{ + Type: v1alpha1.TypeReady, + Status: metav1.ConditionFalse, + LastTransitionTime: metav1.Now(), + Reason: reason, + Message: message, + } +} + +// ReadyUnknown returns a condition that indicates the resource readiness is unknown +func ReadyUnknown(reason, message string) metav1.Condition { + return metav1.Condition{ + Type: v1alpha1.TypeReady, + Status: metav1.ConditionUnknown, + LastTransitionTime: metav1.Now(), + Reason: reason, + Message: message, + } +} + // Error returns a condition that indicates a unspecified error has occurred func Error(message string) metav1.Condition { return metav1.Condition{ diff --git a/internal/controller/federatedmanagedmetric_controller.go b/internal/controller/federatedmanagedmetric_controller.go index b16dbd3..6de552c 100644 --- a/internal/controller/federatedmanagedmetric_controller.go +++ b/internal/controller/federatedmanagedmetric_controller.go @@ -23,6 +23,7 @@ import ( "github.com/go-logr/logr" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" @@ -125,6 +126,18 @@ func (r *FederatedManagedMetricReconciler) Reconcile(ctx context.Context, req ct return r.handleGetError(errLoad, l) } + // Defer status update to ensure it's always called + defer func() { + if err := r.getClient().Status().Update(ctx, &metric); err != nil { + l.Error(err, "Failed to update FederatedManagedMetric status") + } + }() + + // Initialize Ready condition if not present + if meta.FindStatusCondition(metric.Status.Conditions, v1alpha1.TypeReady) == nil { + metric.SetConditions(common.ReadyUnknown("Reconciling", "Initial reconciliation")) + } + // Check if enough time has passed since the last reconciliation if !r.shouldReconcile(&metric) { return r.scheduleNextReconciliation(&metric) @@ -135,6 +148,8 @@ func (r *FederatedManagedMetricReconciler) Reconcile(ctx context.Context, req ct */ credentials, errCredentials := r.getDataSinkCredentials(ctx, &metric, l) if errCredentials != nil { + metric.SetConditions(common.ReadyFalse("DataSinkUnavailable", errCredentials.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse return ctrl.Result{RequeueAfter: RequeueAfterError}, errCredentials } @@ -143,6 +158,8 @@ func (r *FederatedManagedMetricReconciler) Reconcile(ctx context.Context, req ct */ queryConfigs, err := config.CreateExternalQueryConfigSet(ctx, metric.Spec.FederatedClusterAccessRef, r.getClient(), r.getRestConfig()) if err != nil { + metric.SetConditions(common.ReadyFalse("QueryConfigCreationFailed", err.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(err, "unable to create query configs") return ctrl.Result{RequeueAfter: RequeueAfterError}, err } @@ -150,6 +167,8 @@ func (r *FederatedManagedMetricReconciler) Reconcile(ctx context.Context, req ct metricClient, errCli := clientoptl.NewMetricClient(ctx, credentials.Host, credentials.Token) if errCli != nil { + metric.SetConditions(common.ReadyFalse("OTLPClientCreationFailed", errCli.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errCli, fmt.Sprintf("federated managed metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) return ctrl.Result{RequeueAfter: RequeueAfterError}, errCli } @@ -165,6 +184,8 @@ func (r *FederatedManagedMetricReconciler) Reconcile(ctx context.Context, req ct gaugeMetric, errGauge := metricClient.NewMetric(metric.Name) if errGauge != nil { + metric.SetConditions(common.ReadyFalse("MetricCreationFailed", errGauge.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errGauge, fmt.Sprintf("federated managed metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) return ctrl.Result{RequeueAfter: RequeueAfterError}, errGauge } @@ -173,6 +194,8 @@ func (r *FederatedManagedMetricReconciler) Reconcile(ctx context.Context, req ct orchestrator, errOrch := orc.NewOrchestrator(credentials, queryConfig).WithFederatedManaged(metric, gaugeMetric) if errOrch != nil { + metric.SetConditions(common.ReadyFalse("OrchestratorCreationFailed", errOrch.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errOrch, "unable to create federate metric orchestrator monitor") r.Recorder.Event(&metric, "Warning", "OrchestratorCreation", "unable to create orchestrator") return ctrl.Result{RequeueAfter: RequeueAfterError}, errOrch @@ -181,6 +204,8 @@ func (r *FederatedManagedMetricReconciler) Reconcile(ctx context.Context, req ct _, errMon := orchestrator.Handler.Monitor(ctx) if errMon != nil { + metric.SetConditions(common.ReadyFalse("MonitoringFailed", errMon.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errMon, fmt.Sprintf("federated managed metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) return ctrl.Result{RequeueAfter: RequeueAfterError}, errMon } @@ -189,22 +214,19 @@ func (r *FederatedManagedMetricReconciler) Reconcile(ctx context.Context, req ct errExport := metricClient.ExportMetrics(ctx) if errExport != nil { - metric.Status.Ready = "False" + metric.SetConditions(common.ReadyFalse("MetricExportFailed", errExport.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errExport, fmt.Sprintf("federated managed metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) } else { - metric.Status.Ready = "True" + metric.SetConditions(common.ReadyTrue("Federated managed metric reconciled successfully")) + metric.Status.Ready = v1alpha1.StatusStringTrue } // Update LastReconcileTime now := metav1.Now() metric.Status.LastReconcileTime = &now - // conditions are not persisted until the status is updated - errUp := r.getClient().Status().Update(ctx, &metric) - if errUp != nil { - l.Error(errUp, fmt.Sprintf("federated managed metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) - return ctrl.Result{RequeueAfter: RequeueAfterError}, errUp - } + // Note: Status update is handled by the defer function at the beginning /* 4. Re-queue the metric after the frequency or 2 minutes if an error occurred diff --git a/internal/controller/federatedmetric_controller.go b/internal/controller/federatedmetric_controller.go index f2e4402..fbc2dce 100644 --- a/internal/controller/federatedmetric_controller.go +++ b/internal/controller/federatedmetric_controller.go @@ -23,6 +23,7 @@ import ( "github.com/go-logr/logr" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" @@ -127,6 +128,18 @@ func (r *FederatedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Requ return handleGetError(errLoad, l) } + // Defer status update to ensure it's always called + defer func() { + if err := r.getClient().Status().Update(ctx, &metric); err != nil { + l.Error(err, "Failed to update FederatedMetric status") + } + }() + + // Initialize Ready condition if not present + if meta.FindStatusCondition(metric.Status.Conditions, v1alpha1.TypeReady) == nil { + metric.SetConditions(common.ReadyUnknown("Reconciling", "Initial reconciliation")) + } + // Check if enough time has passed since the last reconciliation if !shouldReconcile(&metric) { return scheduleNextReconciliation(&metric) @@ -137,6 +150,8 @@ func (r *FederatedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Requ */ credentials, err := r.getDataSinkCredentials(ctx, &metric, l) if err != nil { + metric.SetConditions(common.ReadyFalse("DataSinkUnavailable", err.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse return ctrl.Result{RequeueAfter: RequeueAfterError}, err } @@ -145,6 +160,8 @@ func (r *FederatedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Requ */ queryConfigs, err := config.CreateExternalQueryConfigSet(ctx, metric.Spec.FederatedClusterAccessRef, r.getClient(), r.getRestConfig()) if err != nil { + metric.SetConditions(common.ReadyFalse("QueryConfigCreationFailed", err.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(err, "unable to create query configs") return ctrl.Result{RequeueAfter: RequeueAfterError}, err } @@ -152,6 +169,8 @@ func (r *FederatedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Requ metricClient, errCli := clientoptl.NewMetricClient(ctx, credentials.Host, credentials.Token) if errCli != nil { + metric.SetConditions(common.ReadyFalse("OTLPClientCreationFailed", errCli.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errCli, fmt.Sprintf("federated metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) return ctrl.Result{RequeueAfter: RequeueAfterError}, errCli } @@ -167,6 +186,8 @@ func (r *FederatedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Requ gaugeMetric, errGauge := metricClient.NewMetric(metric.Name) if errGauge != nil { + metric.SetConditions(common.ReadyFalse("MetricCreationFailed", errGauge.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errGauge, fmt.Sprintf("federated metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) return ctrl.Result{RequeueAfter: RequeueAfterError}, errGauge } @@ -175,6 +196,8 @@ func (r *FederatedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Requ orchestrator, errOrch := orc.NewOrchestrator(credentials, queryConfig).WithFederated(metric, gaugeMetric) if errOrch != nil { + metric.SetConditions(common.ReadyFalse("OrchestratorCreationFailed", errOrch.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errOrch, "unable to create federate metric orchestrator monitor") r.Recorder.Event(&metric, "Warning", "OrchestratorCreation", "unable to create orchestrator") return ctrl.Result{RequeueAfter: RequeueAfterError}, errOrch @@ -183,6 +206,8 @@ func (r *FederatedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Requ _, errMon := orchestrator.Handler.Monitor(ctx) if errMon != nil { + metric.SetConditions(common.ReadyFalse("MonitoringFailed", errMon.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errMon, fmt.Sprintf("federated metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) return ctrl.Result{RequeueAfter: RequeueAfterError}, errMon } @@ -191,22 +216,19 @@ func (r *FederatedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Requ errExport := metricClient.ExportMetrics(ctx) if errExport != nil { - metric.Status.Ready = "False" + metric.SetConditions(common.ReadyFalse("MetricExportFailed", errExport.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errExport, fmt.Sprintf("federated metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) } else { - metric.Status.Ready = "True" + metric.SetConditions(common.ReadyTrue("Federated metric reconciled successfully")) + metric.Status.Ready = v1alpha1.StatusStringTrue } // Update LastReconcileTime now := metav1.Now() metric.Status.LastReconcileTime = &now - // conditions are not persisted until the status is updated - errUp := r.getClient().Status().Update(ctx, &metric) - if errUp != nil { - l.Error(errUp, fmt.Sprintf("generic metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) - return ctrl.Result{RequeueAfter: RequeueAfterError}, errUp - } + // Note: Status update is handled by the defer function at the beginning /* 4. Requeue the metric after the frequency or after 2 minutes if an error occurred diff --git a/internal/controller/managedmetric_controller.go b/internal/controller/managedmetric_controller.go index c90a12b..6702b6f 100644 --- a/internal/controller/managedmetric_controller.go +++ b/internal/controller/managedmetric_controller.go @@ -24,6 +24,7 @@ import ( "time" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" @@ -123,6 +124,18 @@ func (r *ManagedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{RequeueAfter: RequeueAfterError}, errLoad } + // Defer status update to ensure it's always called + defer func() { + if err := r.inClient.Status().Update(ctx, &metric); err != nil { + l.Error(err, "Failed to update ManagedMetric status") + } + }() + + // Initialize Ready condition if not present + if meta.FindStatusCondition(metric.Status.Conditions, v1alpha1.TypeReady) == nil { + metric.SetConditions(common.ReadyUnknown("Reconciling", "Initial reconciliation")) + } + // Check if enough time has passed since the last reconciliation if !r.shouldReconcile(&metric) { return r.scheduleNextReconciliation(&metric) @@ -133,6 +146,8 @@ func (r *ManagedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Reques */ credentials, err := r.getDataSinkCredentials(ctx, &metric, l) if err != nil { + metric.SetConditions(common.ReadyFalse("DataSinkUnavailable", err.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse return ctrl.Result{RequeueAfter: RequeueAfterError}, err } @@ -141,6 +156,8 @@ func (r *ManagedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Reques */ queryConfig, err := createQueryConfig(ctx, metric.Spec.RemoteClusterAccessRef, r) if err != nil { + metric.SetConditions(common.ReadyFalse("QueryConfigCreationFailed", err.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse return ctrl.Result{RequeueAfter: RequeueAfterError}, err } @@ -149,6 +166,8 @@ func (r *ManagedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Reques */ metricClient, errCli := clientoptl.NewMetricClient(ctx, credentials.Host, credentials.Token) if errCli != nil { + metric.SetConditions(common.ReadyFalse("OTLPClientCreationFailed", errCli.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errCli, fmt.Sprintf("managed metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) return ctrl.Result{RequeueAfter: RequeueAfterError}, errCli } @@ -164,6 +183,8 @@ func (r *ManagedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Reques gaugeMetric, errGauge := metricClient.NewMetric(metric.Name) if errGauge != nil { + metric.SetConditions(common.ReadyFalse("MetricCreationFailed", errGauge.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errGauge, fmt.Sprintf("managed metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) return ctrl.Result{RequeueAfter: RequeueAfterError}, errGauge } @@ -173,6 +194,8 @@ func (r *ManagedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Reques */ orchestrator, errOrch := orchestrator.NewOrchestrator(credentials, queryConfig).WithManaged(metric, gaugeMetric) if errOrch != nil { + metric.SetConditions(common.ReadyFalse("OrchestratorCreationFailed", errOrch.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errOrch, "unable to create managed metric orchestrator monitor") r.Recorder.Event(&metric, "Warning", "OrchestratorCreation", "unable to create orchestrator") return ctrl.Result{RequeueAfter: RequeueAfterError}, errOrch @@ -181,6 +204,8 @@ func (r *ManagedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Reques result, errMon := orchestrator.Handler.Monitor(ctx) if errMon != nil { + metric.SetConditions(common.ReadyFalse("MonitoringFailed", errMon.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errMon, fmt.Sprintf("managed metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) return ctrl.Result{RequeueAfter: RequeueAfterError}, errMon } @@ -189,12 +214,6 @@ func (r *ManagedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Reques 2.1 Export metrics to data sink */ errExport := metricClient.ExportMetrics(ctx) - if errExport != nil { - metric.Status.Ready = v1alpha1.StatusFalse - l.Error(errExport, fmt.Sprintf("managed metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) - } else { - metric.Status.Ready = v1alpha1.StatusTrue - } /* 3. Update the status of the metric with conditions and phase @@ -212,9 +231,14 @@ func (r *ManagedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Reques r.Recorder.Event(&metric, "Normal", "MetricPending", result.Message) } - // Override Ready status if export failed + // Set Ready condition based on export result if errExport != nil { - metric.Status.Ready = v1alpha1.StatusFalse + metric.SetConditions(common.ReadyFalse("MetricExportFailed", errExport.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse + l.Error(errExport, fmt.Sprintf("managed metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) + } else { + metric.SetConditions(common.ReadyTrue("Managed metric reconciled successfully")) + metric.Status.Ready = v1alpha1.StatusStringTrue } // Update the observation timestamp to track when this reconciliation happened @@ -223,12 +247,7 @@ func (r *ManagedMetricReconciler) Reconcile(ctx context.Context, req ctrl.Reques Resources: result.Observation.GetValue(), } - // conditions are not persisted until the status is updated - errUp := r.inClient.Status().Update(ctx, &metric) - if errUp != nil { - l.Error(errUp, fmt.Sprintf("managed metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) - return ctrl.Result{RequeueAfter: RequeueAfterError}, errUp - } + // Note: Status update is handled by the defer function at the beginning /* 4. Requeue the metric after the frequency or after 2 minutes if an error occurred diff --git a/internal/controller/metric_controller.go b/internal/controller/metric_controller.go index 9ce0fdc..f18f293 100644 --- a/internal/controller/metric_controller.go +++ b/internal/controller/metric_controller.go @@ -23,6 +23,7 @@ import ( "github.com/go-logr/logr" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" @@ -132,6 +133,18 @@ func (r *MetricReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return r.handleGetError(errLoad, l) } + // Defer status update to ensure it's always called + defer func() { + if err := r.getClient().Status().Update(ctx, &metric); err != nil { + l.Error(err, "Failed to update Metric status") + } + }() + + // Initialize Ready condition if not present + if meta.FindStatusCondition(metric.Status.Conditions, v1alpha1.TypeReady) == nil { + metric.SetConditions(common.ReadyUnknown("Reconciling", "Initial reconciliation")) + } + // Check if enough time has passed since the last reconciliation if !r.shouldReconcile(&metric) { return r.scheduleNextReconciliation(&metric) @@ -142,6 +155,8 @@ func (r *MetricReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr */ credentials, err := r.getDataSinkCredentials(ctx, &metric, l) if err != nil { + metric.SetConditions(common.ReadyFalse("DataSinkUnavailable", err.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse return ctrl.Result{RequeueAfter: RequeueAfterError}, err } @@ -150,13 +165,16 @@ func (r *MetricReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr */ queryConfig, err := createQC(ctx, metric.Spec.RemoteClusterAccessRef, r) if err != nil { + metric.SetConditions(common.ReadyFalse("QueryConfigCreationFailed", err.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse return ctrl.Result{RequeueAfter: RequeueAfterError}, err } metricClient, errCli := clientoptl.NewMetricClient(ctx, credentials.Host, credentials.Token) if errCli != nil { + metric.SetConditions(common.ReadyFalse("OTLPClientCreationFailed", errCli.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errCli, fmt.Sprintf("metric '%s' failed to create OTel client, re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) - // TODO: Update status? return ctrl.Result{RequeueAfter: RequeueAfterError}, errCli } defer func() { @@ -169,8 +187,9 @@ func (r *MetricReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr gaugeMetric, errGauge := metricClient.NewMetric(metric.Name) if errGauge != nil { + metric.SetConditions(common.ReadyFalse("MetricCreationFailed", errGauge.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errGauge, fmt.Sprintf("metric '%s' failed to create OTel gauge, re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) - // TODO: Update status? return ctrl.Result{RequeueAfter: RequeueAfterError}, errGauge } /* @@ -178,6 +197,8 @@ func (r *MetricReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr */ orchestrator, errOrch := orc.NewOrchestrator(credentials, queryConfig).WithMetric(metric, gaugeMetric) // Pass gaugeMetric if errOrch != nil { + metric.SetConditions(common.ReadyFalse("OrchestratorCreationFailed", errOrch.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errOrch, "unable to create metric orchestrator monitor") r.Recorder.Event(&metric, "Warning", "OrchestratorCreation", "unable to create orchestrator") return ctrl.Result{RequeueAfter: RequeueAfterError}, errOrch @@ -186,20 +207,13 @@ func (r *MetricReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr result, errMon := orchestrator.Handler.Monitor(ctx) if errMon != nil { - metric.Status.Ready = v1alpha1.StatusFalse + metric.SetConditions(common.ReadyFalse("MonitoringFailed", errMon.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse l.Error(errMon, fmt.Sprintf("metric '%s' re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) - // Update status before returning - _ = r.getClient().Status().Update(ctx, &metric) // Best effort status update on error return ctrl.Result{RequeueAfter: RequeueAfterError}, errMon } errExport := metricClient.ExportMetrics(ctx) - if errExport != nil { - metric.Status.Ready = v1alpha1.StatusFalse - l.Error(errExport, fmt.Sprintf("metric '%s' failed to export, re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) - } else { - metric.Status.Ready = v1alpha1.StatusTrue - } /* 3. Update the status of the metric with conditions and phase @@ -219,11 +233,16 @@ func (r *MetricReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr cObs := result.Observation.(*v1alpha1.MetricObservation) - // Override Ready status if export failed + // Set Ready condition based on export result if errExport != nil { - metric.Status.Ready = "False" + metric.SetConditions(common.ReadyFalse("MetricExportFailed", errExport.Error())) + metric.Status.Ready = v1alpha1.StatusStringFalse + l.Error(errExport, fmt.Sprintf("metric '%s' failed to export, re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) + } else { + metric.SetConditions(common.ReadyTrue("Metric reconciled successfully")) + metric.Status.Ready = v1alpha1.StatusStringTrue } - // metric.Status.Observation = v1beta1.MetricObservation{Timestamp: result.Observation.GetTimestamp(), Dimensions: cObs.Dimensions, LatestValue: cObs.LatestValue} + metric.Status.Observation = v1alpha1.MetricObservation{ Timestamp: result.Observation.GetTimestamp(), LatestValue: cObs.LatestValue, @@ -233,12 +252,7 @@ func (r *MetricReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // Update LastReconcileTime metric.Status.Observation.Timestamp.Time = metav1.Now().Time - // conditions are not persisted until the status is updated - errUp := r.getClient().Status().Update(ctx, &metric) - if errUp != nil { - l.Error(errUp, fmt.Sprintf("metric '%s' failed to update status, re-queued for execution in %v minutes\n", metric.Spec.Name, RequeueAfterError)) - return ctrl.Result{RequeueAfter: RequeueAfterError}, errUp - } + // Note: Status update is handled by the defer function at the beginning /* 4. Requeue the metric after the frequency or after 2 minutes if an error occurred From 4eff7ec8e68b1113c61340ffb895cb787409ea48 Mon Sep 17 00:00:00 2001 From: Mirza Kopic Date: Mon, 2 Jun 2025 23:15:04 +0200 Subject: [PATCH 4/5] fix make reviewable for now --- internal/controller/datasink_utils.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/controller/datasink_utils.go b/internal/controller/datasink_utils.go index 0cc4095..819b1ed 100644 --- a/internal/controller/datasink_utils.go +++ b/internal/controller/datasink_utils.go @@ -47,6 +47,8 @@ func NewDataSinkCredentialsRetriever(client client.Client, recorder record.Event } // GetDataSinkCredentials fetches DataSink configuration and credentials for any metric type +// +//nolint:gocyclo func (d *DataSinkCredentialsRetriever) GetDataSinkCredentials(ctx context.Context, dataSinkRef *v1alpha1.DataSinkReference, eventObject client.Object, l logr.Logger) (common.DataSinkCredentials, error) { // Determine the namespace where DataSinks are expected to be found. dataSinkLookupNamespace := os.Getenv("OPERATOR_CONFIG_NAMESPACE") From 7d21761b2afe04ac7700e52a6c949ae89c17237e Mon Sep 17 00:00:00 2001 From: Mirza Kopic Date: Mon, 2 Jun 2025 23:30:54 +0200 Subject: [PATCH 5/5] fix helmchart --- charts/metrics-operator/values.yaml | 62 +++++------------------------ 1 file changed, 10 insertions(+), 52 deletions(-) diff --git a/charts/metrics-operator/values.yaml b/charts/metrics-operator/values.yaml index 4606f0e..d61a872 100644 --- a/charts/metrics-operator/values.yaml +++ b/charts/metrics-operator/values.yaml @@ -107,68 +107,26 @@ webhooks: rbac: clusterRole: rules: - - apiGroups: - - "" - resources: - - secrets - verbs: - - get - - list - - watch - - apiGroups: - - "" - resources: - - events - verbs: - - create - - patch - apiGroups: - metrics.cloud.sap resources: - - clusteraccesses - - datasinks - - federatedclusteraccesses - - federatedmanagedmetrics - - federatedmetrics - managedmetrics - metrics - - remoteclusteraccesses - verbs: - - create - - delete - - get - - list - - patch - - update - - watch - - apiGroups: - - metrics.cloud.sap - resources: - - clusteraccesses/finalizers - - datasinks/finalizers - - federatedclusteraccesses/finalizers - - federatedmanagedmetrics/finalizers - - federatedmetrics/finalizers - - managedmetrics/finalizers - - metrics/finalizers - - remoteclusteraccesses/finalizers + - metrics/status + - managedmetrics/status verbs: - - update + - "*" - apiGroups: - - metrics.cloud.sap + - "" resources: - - clusteraccesses/status - - datasinks/status - - federatedclusteraccesses/status - - federatedmanagedmetrics/status - - federatedmetrics/status - - managedmetrics/status - - metrics/status - - remoteclusteraccesses/status + - secrets verbs: - get - - patch - - update + - list + - watch + - apiGroups: [ "*" ] + resources: [ "*" ] + verbs: [ "get", "list", "watch" ] role: rules: []