Merge pull request #476 from openshift-cherrypick-robot/cherry-pick-4…

…24-to-18.0.0-proposed [18.0.0-proposed] [OSPRH-8076] Fix CustomMonitoringStack
openstack-k8s-operators · Sep 20, 2024 · 2e89e59 · 2e89e59
2 parents 2e54f59 + fa62956
commit 2e89e59
Show file tree

Hide file tree

Showing 13 changed files with 211 additions and 68 deletions.
diff --git a/api/bases/telemetry.openstack.org_metricstorages.yaml b/api/bases/telemetry.openstack.org_metricstorages.yaml
@@ -1069,6 +1069,17 @@ spec:
                     pattern: ^(0|(([0-9]+)y)?(([0-9]+)w)?(([0-9]+)d)?(([0-9]+)h)?(([0-9]+)m)?(([0-9]+)s)?(([0-9]+)ms)?)$
                     type: string
                 type: object
+              dashboardsEnabled:
+                default: false
+                description: DashboardsEnabled allows to enable or disable dashboards
+                  and related artifacts
+                type: boolean
+              dataplaneNetwork:
+                default: ctlplane
+                description: DataplaneNetwork defines the network that will be used
+                  to scrape dataplane node_exporter endpoints
+                pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$
+                type: string
               monitoringStack:
                 description: MonitoringStack allows to define a metric storage with
                   options supported by Red Hat
@@ -1078,17 +1089,6 @@ spec:
                     default: true
                     description: AlertingEnabled allows to enable or disable alertmanager
                     type: boolean
-                  dashboardsEnabled:
-                    default: false
-                    description: DashboardsEnabled allows to enable or disable dashboards
-                      and related artifacts
-                    type: boolean
-                  dataplaneNetwork:
-                    default: ctlplane
-                    description: DataplaneNetwork defines the network that will be
-                      used to scrape dataplane node_exporter endpoints
-                    pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$
-                    type: string
                   scrapeInterval:
                     default: 30s
                     description: ScrapeInterval sets the interval between scrapes
@@ -1175,8 +1175,6 @@ spec:
                         - persistent
                         type: string
                     type: object
-                required:
-                - dataplaneNetwork
                 type: object
               prometheusTls:
                 description: TLS - Parameters related to the TLS
@@ -1189,6 +1187,8 @@ spec:
                     description: SecretName - holding the cert, key for the service
                     type: string
                 type: object
+            required:
+            - dataplaneNetwork
             type: object
           status:
             description: MetricStorageStatus defines the observed state of MetricStorage

diff --git a/api/bases/telemetry.openstack.org_telemetries.yaml b/api/bases/telemetry.openstack.org_telemetries.yaml
@@ -1597,6 +1597,17 @@ spec:
                         pattern: ^(0|(([0-9]+)y)?(([0-9]+)w)?(([0-9]+)d)?(([0-9]+)h)?(([0-9]+)m)?(([0-9]+)s)?(([0-9]+)ms)?)$
                         type: string
                     type: object
+                  dashboardsEnabled:
+                    default: false
+                    description: DashboardsEnabled allows to enable or disable dashboards
+                      and related artifacts
+                    type: boolean
+                  dataplaneNetwork:
+                    default: ctlplane
+                    description: DataplaneNetwork defines the network that will be
+                      used to scrape dataplane node_exporter endpoints
+                    pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$
+                    type: string
                   enabled:
                     default: false
                     description: Enabled - Whether a MetricStorage should be deployed
@@ -1611,17 +1622,6 @@ spec:
                         default: true
                         description: AlertingEnabled allows to enable or disable alertmanager
                         type: boolean
-                      dashboardsEnabled:
-                        default: false
-                        description: DashboardsEnabled allows to enable or disable
-                          dashboards and related artifacts
-                        type: boolean
-                      dataplaneNetwork:
-                        default: ctlplane
-                        description: DataplaneNetwork defines the network that will
-                          be used to scrape dataplane node_exporter endpoints
-                        pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$
-                        type: string
                       scrapeInterval:
                         default: 30s
                         description: ScrapeInterval sets the interval between scrapes
@@ -1710,8 +1710,6 @@ spec:
                             - persistent
                             type: string
                         type: object
-                    required:
-                    - dataplaneNetwork
                     type: object
                   prometheusTls:
                     description: TLS - Parameters related to the TLS
@@ -1724,6 +1722,8 @@ spec:
                         description: SecretName - holding the cert, key for the service
                         type: string
                     type: object
+                required:
+                - dataplaneNetwork
                 type: object
             type: object
           status:

diff --git a/api/v1beta1/metricstorage_types.go b/api/v1beta1/metricstorage_types.go
@@ -67,20 +67,11 @@ type MonitoringStack struct {
 	// +kubebuilder:default=true
 	AlertingEnabled bool `json:"alertingEnabled"`
 
-	// DashboardsEnabled allows to enable or disable dashboards and related artifacts
-	// +kubebuilder:validation:Optional
-	// +kubebuilder:default=false
-	DashboardsEnabled bool `json:"dashboardsEnabled"`
-
 	// ScrapeInterval sets the interval between scrapes
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default="30s"
 	ScrapeInterval string `json:"scrapeInterval"`
 
-	// DataplaneNetwork defines the network that will be used to scrape dataplane node_exporter endpoints
-	// +kubebuilder:default=ctlplane
-	DataplaneNetwork infranetworkv1.NetNameStr `json:"dataplaneNetwork"`
-
 	// Storage allows to define options for how to store metrics
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default={strategy: persistent, retention: "24h", persistent: {pvcStorageRequest: "20G"}}
@@ -89,6 +80,15 @@ type MonitoringStack struct {
 
 // MetricStorageSpec defines the desired state of MetricStorage
 type MetricStorageSpec struct {
+	// DashboardsEnabled allows to enable or disable dashboards and related artifacts
+	// +kubebuilder:validation:Optional
+	// +kubebuilder:default=false
+	DashboardsEnabled bool `json:"dashboardsEnabled"`
+
+	// DataplaneNetwork defines the network that will be used to scrape dataplane node_exporter endpoints
+	// +kubebuilder:default=ctlplane
+	DataplaneNetwork infranetworkv1.NetNameStr `json:"dataplaneNetwork"`
+
 	// MonitoringStack allows to define a metric storage with
 	// options supported by Red Hat
 	// +kubebuilder:validation:Optional

diff --git a/api/v1beta1/metricstorage_webhook.go b/api/v1beta1/metricstorage_webhook.go
@@ -55,6 +55,9 @@ func (spec *MetricStorageSpec) Default() {
 		// NOTE: If we want to enable dashboards in the future by default, set
 		//       it here like Alerting above
 	}
+	if spec.DataplaneNetwork == "" {
+		spec.DataplaneNetwork = "ctlplane"
+	}
 	if spec.MonitoringStack != nil {
 		spec.MonitoringStack.Default()
 	}
@@ -65,9 +68,6 @@ func (ms *MonitoringStack) Default() {
 	if ms.ScrapeInterval == "" {
 		ms.ScrapeInterval = "30s"
 	}
-	if ms.DataplaneNetwork == "" {
-		ms.DataplaneNetwork = "ctlplane"
-	}
 	ms.Storage.Default()
 }
 

diff --git a/config/crd/bases/telemetry.openstack.org_metricstorages.yaml b/config/crd/bases/telemetry.openstack.org_metricstorages.yaml
@@ -1069,6 +1069,17 @@ spec:
                     pattern: ^(0|(([0-9]+)y)?(([0-9]+)w)?(([0-9]+)d)?(([0-9]+)h)?(([0-9]+)m)?(([0-9]+)s)?(([0-9]+)ms)?)$
                     type: string
                 type: object
+              dashboardsEnabled:
+                default: false
+                description: DashboardsEnabled allows to enable or disable dashboards
+                  and related artifacts
+                type: boolean
+              dataplaneNetwork:
+                default: ctlplane
+                description: DataplaneNetwork defines the network that will be used
+                  to scrape dataplane node_exporter endpoints
+                pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$
+                type: string
               monitoringStack:
                 description: MonitoringStack allows to define a metric storage with
                   options supported by Red Hat
@@ -1078,17 +1089,6 @@ spec:
                     default: true
                     description: AlertingEnabled allows to enable or disable alertmanager
                     type: boolean
-                  dashboardsEnabled:
-                    default: false
-                    description: DashboardsEnabled allows to enable or disable dashboards
-                      and related artifacts
-                    type: boolean
-                  dataplaneNetwork:
-                    default: ctlplane
-                    description: DataplaneNetwork defines the network that will be
-                      used to scrape dataplane node_exporter endpoints
-                    pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$
-                    type: string
                   scrapeInterval:
                     default: 30s
                     description: ScrapeInterval sets the interval between scrapes
@@ -1175,8 +1175,6 @@ spec:
                         - persistent
                         type: string
                     type: object
-                required:
-                - dataplaneNetwork
                 type: object
               prometheusTls:
                 description: TLS - Parameters related to the TLS
@@ -1189,6 +1187,8 @@ spec:
                     description: SecretName - holding the cert, key for the service
                     type: string
                 type: object
+            required:
+            - dataplaneNetwork
             type: object
           status:
             description: MetricStorageStatus defines the observed state of MetricStorage

diff --git a/config/crd/bases/telemetry.openstack.org_telemetries.yaml b/config/crd/bases/telemetry.openstack.org_telemetries.yaml
@@ -1597,6 +1597,17 @@ spec:
                         pattern: ^(0|(([0-9]+)y)?(([0-9]+)w)?(([0-9]+)d)?(([0-9]+)h)?(([0-9]+)m)?(([0-9]+)s)?(([0-9]+)ms)?)$
                         type: string
                     type: object
+                  dashboardsEnabled:
+                    default: false
+                    description: DashboardsEnabled allows to enable or disable dashboards
+                      and related artifacts
+                    type: boolean
+                  dataplaneNetwork:
+                    default: ctlplane
+                    description: DataplaneNetwork defines the network that will be
+                      used to scrape dataplane node_exporter endpoints
+                    pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$
+                    type: string
                   enabled:
                     default: false
                     description: Enabled - Whether a MetricStorage should be deployed
@@ -1611,17 +1622,6 @@ spec:
                         default: true
                         description: AlertingEnabled allows to enable or disable alertmanager
                         type: boolean
-                      dashboardsEnabled:
-                        default: false
-                        description: DashboardsEnabled allows to enable or disable
-                          dashboards and related artifacts
-                        type: boolean
-                      dataplaneNetwork:
-                        default: ctlplane
-                        description: DataplaneNetwork defines the network that will
-                          be used to scrape dataplane node_exporter endpoints
-                        pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$
-                        type: string
                       scrapeInterval:
                         default: 30s
                         description: ScrapeInterval sets the interval between scrapes
@@ -1710,8 +1710,6 @@ spec:
                             - persistent
                             type: string
                         type: object
-                    required:
-                    - dataplaneNetwork
                     type: object
                   prometheusTls:
                     description: TLS - Parameters related to the TLS
@@ -1724,6 +1722,8 @@ spec:
                         description: SecretName - holding the cert, key for the service
                         type: string
                     type: object
+                required:
+                - dataplaneNetwork
                 type: object
             type: object
           status:

diff --git a/controllers/metricstorage_controller.go b/controllers/metricstorage_controller.go
@@ -522,7 +522,7 @@ func (r *MetricStorageReconciler) reconcileNormal(
 	}
 	instance.Status.Conditions.MarkTrue(telemetryv1.ScrapeConfigReadyCondition, condition.ReadyMessage)
 
-	if !instance.Spec.MonitoringStack.DashboardsEnabled {
+	if !instance.Spec.DashboardsEnabled {
 		if res, err := metricstorage.DeleteDashboardObjects(ctx, instance, helper); err != nil {
 			return res, err
 		}
@@ -874,7 +874,7 @@ func getAddressFromIPSet(
 	if len(ipset.Status.Reservation) > 0 {
 		// search for the network specified in the Spec
 		for _, reservation := range ipset.Status.Reservation {
-			if reservation.Network == instance.Spec.MonitoringStack.DataplaneNetwork {
+			if reservation.Network == instance.Spec.DataplaneNetwork {
 				return reservation.Address, discoveryv1.AddressTypeIPv4
 			}
 		}

diff --git a/controllers/telemetry_controller.go b/controllers/telemetry_controller.go
@@ -409,10 +409,14 @@ func (r TelemetryReconciler) reconcileMetricStorage(ctx context.Context, instanc
 		if instance.Spec.MetricStorage.MetricStorageSpec.CustomMonitoringStack != nil {
 			metricStorageInstance.Spec.CustomMonitoringStack = &obov1.MonitoringStackSpec{}
 			instance.Spec.MetricStorage.MetricStorageSpec.CustomMonitoringStack.DeepCopyInto(metricStorageInstance.Spec.CustomMonitoringStack)
+		} else {
+			metricStorageInstance.Spec.CustomMonitoringStack = nil
 		}
 		if instance.Spec.MetricStorage.MetricStorageSpec.MonitoringStack != nil {
 			metricStorageInstance.Spec.MonitoringStack = &telemetryv1.MonitoringStack{}
 			instance.Spec.MetricStorage.MetricStorageSpec.MonitoringStack.DeepCopyInto(metricStorageInstance.Spec.MonitoringStack)
+		} else {
+			metricStorageInstance.Spec.MonitoringStack = nil
 		}
 		instance.Spec.MetricStorage.MetricStorageSpec.PrometheusTLS.DeepCopyInto(&metricStorageInstance.Spec.PrometheusTLS)
 		// TODO: Uncomment this line when implementing TLS for Alertmanager

diff --git a/tests/kuttl/suites/metricstorage/tests/01-deploy.yaml b/tests/kuttl/suites/metricstorage/tests/01-deploy.yaml
@@ -3,9 +3,9 @@ kind: MetricStorage
 metadata:
   name: telemetry-kuttl
 spec:
+  dashboardsEnabled: true
   monitoringStack:
     alertingEnabled: true
-    dashboardsEnabled: true
     scrapeInterval: 30s
     storage:
       strategy: persistent

diff --git a/tests/kuttl/suites/metricstorage/tests/02-disable-dashboards.yaml b/tests/kuttl/suites/metricstorage/tests/02-disable-dashboards.yaml
@@ -3,9 +3,9 @@ kind: MetricStorage
 metadata:
   name: telemetry-kuttl
 spec:
+  dashboardsEnabled: false
   monitoringStack:
     alertingEnabled: true
-    dashboardsEnabled: false
     scrapeInterval: 30s
     storage:
       strategy: persistent

diff --git a/tests/kuttl/suites/metricstorage/tests/03-prepare-for-custom-monitoring-stack.yaml b/tests/kuttl/suites/metricstorage/tests/03-prepare-for-custom-monitoring-stack.yaml
@@ -0,0 +1,12 @@
+# Kuttl test doesn't seem to wait correctly for the metric
+# storage to reconcile in the next test step when switching
+# to CustomMonitoringStack (in the output of the test I
+# can see it applyes the resource and fails a second later).
+# Deleting it here and than recreating it in the next step
+# should ensure kuttl waits long enough.
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+delete:
+- apiVersion: telemetry.openstack.org/v1beta1
+  kind: MetricStorage
+  name: telemetry-kuttl