Update some logic

Author: benjaminjb

internal/collector/instance.go

@@ -48,7 +48,9 @@ func AddToPod(
 	dir string,
 	includeLogrotate bool,
 ) {
-	if !(feature.Enabled(ctx, feature.OpenTelemetryLogs) || feature.Enabled(ctx, feature.OpenTelemetryMetrics)) {
+	if spec == nil ||
+		!(feature.Enabled(ctx, feature.OpenTelemetryLogs) ||
+			feature.Enabled(ctx, feature.OpenTelemetryMetrics)) {
 		return
 	}
 

internal/controller/postgrescluster/instance.go

@@ -1168,7 +1168,7 @@ func (r *Reconciler) reconcileInstance(
 	)
 
 	if err == nil {
-		instanceConfigMap, err = r.reconcileInstanceConfigMap(ctx, cluster, spec, instance, otelConfig, backupsSpecFound)
+		instanceConfigMap, err = r.reconcileInstanceConfigMap(ctx, cluster, spec, instance, otelConfig)
 	}
 	if err == nil {
 		instanceCertificates, err = r.reconcileInstanceCertificates(
@@ -1200,27 +1200,28 @@ func (r *Reconciler) reconcileInstance(
 			spec, instanceCertificates, instanceConfigMap, &instance.Spec.Template)
 	}
 
-	if err == nil {
+	// If either OTel feature gates are enabled
+	if err == nil &&
+		(feature.Enabled(ctx, feature.OpenTelemetryLogs) ||
+			feature.Enabled(ctx, feature.OpenTelemetryMetrics)) {
+		{
 
-		// If OTel metrics are enabled, we'll need the MonitoringUserSecret
-		var password = ""
-		if feature.Enabled(ctx, feature.OpenTelemetryMetrics) {
-			monitoringUserSecret := &corev1.Secret{ObjectMeta: naming.MonitoringUserSecret(cluster)}
-			err = errors.WithStack(
-				r.Client.Get(ctx, client.ObjectKeyFromObject(monitoringUserSecret), monitoringUserSecret))
+			// If OTel metrics are enabled, we'll need the MonitoringUserSecret
+			var password = ""
+			if feature.Enabled(ctx, feature.OpenTelemetryMetrics) {
+				monitoringUserSecret := &corev1.Secret{ObjectMeta: naming.MonitoringUserSecret(cluster)}
+				err = errors.WithStack(
+					r.Client.Get(ctx, client.ObjectKeyFromObject(monitoringUserSecret), monitoringUserSecret))
 
-			if err == nil {
-				password = string(monitoringUserSecret.Data["password"])
+				if err == nil {
+					password = string(monitoringUserSecret.Data["password"])
+				}
 			}
-		}
 
-		// FIXME: is this bool really asking for a backup vol?
-		var logrotate = false
-		if backupsSpecFound {
-			logrotate = true
-		}
-
-		if feature.Enabled(ctx, feature.OpenTelemetryLogs) || feature.Enabled(ctx, feature.OpenTelemetryMetrics) {
+			var logrotate = false
+			if pgbackrest.RepoHostVolumeDefined(cluster) {
+				logrotate = true
+			}
 
 			// TODO: Add postgres/patroni dir if we want that.
 			collector.AddToPod(ctx, cluster.Spec.Instrumentation, cluster.Spec.ImagePullPolicy,
@@ -1411,7 +1412,7 @@ func addPGBackRestToInstancePodSpec(
 // files (etc) that apply to instance of cluster.
 func (r *Reconciler) reconcileInstanceConfigMap(
 	ctx context.Context, cluster *v1beta1.PostgresCluster, spec *v1beta1.PostgresInstanceSetSpec,
-	instance *appsv1.StatefulSet, otelConfig *collector.Config, backupSpecFound bool,
+	instance *appsv1.StatefulSet, otelConfig *collector.Config,
 ) (*corev1.ConfigMap, error) {
 	instanceConfigMap := &corev1.ConfigMap{ObjectMeta: naming.InstanceConfigMap(instance)}
 	instanceConfigMap.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ConfigMap"))
@@ -1432,12 +1433,18 @@ func (r *Reconciler) reconcileInstanceConfigMap(
 		})
 
 	// If OTel logging or metrics is enabled, add collector config
-	if err == nil && (feature.Enabled(ctx, feature.OpenTelemetryLogs) || feature.Enabled(ctx, feature.OpenTelemetryMetrics)) {
+	if err == nil &&
+		(feature.Enabled(ctx, feature.OpenTelemetryLogs) ||
+			feature.Enabled(ctx, feature.OpenTelemetryMetrics)) {
 		err = collector.AddToConfigMap(ctx, otelConfig, instanceConfigMap)
 
-		// Add pgbackrest logrotate if OpenTelemetryLogs is enabled and backupSpecFound
-		if err == nil && feature.Enabled(ctx, feature.OpenTelemetryLogs) &&
-			backupSpecFound && cluster.Spec.Instrumentation != nil {
+		// Add pgbackrest logrotate if OpenTelemetryLogs is enabled and
+		// local volumes are available
+		if err == nil &&
+			feature.Enabled(ctx, feature.OpenTelemetryLogs) &&
+			pgbackrest.RepoHostVolumeDefined(cluster) &&
+			cluster.Spec.Instrumentation != nil {
+
 			err = collector.AddLogrotateConfig(ctx, cluster.Spec.Instrumentation,
 				instanceConfigMap, naming.PGBackRestPGDataLogPath+"/*.log", "")
 		}

internal/controller/postgrescluster/pgbouncer.go

@@ -100,7 +100,8 @@ func (r *Reconciler) reconcilePGBouncerConfigMap(
 	}
 	// If OTel logging or metrics is enabled, add collector config
 	if otelConfig != nil &&
-		(feature.Enabled(ctx, feature.OpenTelemetryLogs) || feature.Enabled(ctx, feature.OpenTelemetryMetrics)) {
+		(feature.Enabled(ctx, feature.OpenTelemetryLogs) ||
+			feature.Enabled(ctx, feature.OpenTelemetryMetrics)) {
 		err = collector.AddToConfigMap(ctx, otelConfig, configmap)
 	}
 	// If OTel logging is enabled, add logrotate config

internal/pgbackrest/config.go

@@ -130,25 +130,30 @@ func CreatePGBackRestConfigMapIntent(ctx context.Context, postgresCluster *v1bet
 				postgresCluster.Spec.Backups.PGBackRest.Global,
 			).String()
 
-		err = collector.AddToConfigMap(ctx, collector.NewConfigForPgBackrestRepoHostPod(
-			ctx,
-			postgresCluster.Spec.Instrumentation,
-			postgresCluster.Spec.Backups.PGBackRest.Repos,
-		), cm)
-
-		// If OTel logging is enabled, add logrotate config for the RepoHost
-		if err == nil &&
-			postgresCluster.Spec.Instrumentation != nil &&
-			feature.Enabled(ctx, feature.OpenTelemetryLogs) {
-			var pgBackRestLogPath string
-			for _, repo := range postgresCluster.Spec.Backups.PGBackRest.Repos {
-				if repo.Volume != nil {
-					pgBackRestLogPath = fmt.Sprintf(naming.PGBackRestRepoLogPath, repo.Name)
-					break
+		if RepoHostVolumeDefined(postgresCluster) &&
+			(feature.Enabled(ctx, feature.OpenTelemetryLogs) ||
+				feature.Enabled(ctx, feature.OpenTelemetryMetrics)) {
+			err = collector.AddToConfigMap(ctx, collector.NewConfigForPgBackrestRepoHostPod(
+				ctx,
+				postgresCluster.Spec.Instrumentation,
+				postgresCluster.Spec.Backups.PGBackRest.Repos,
+			), cm)
+
+			// If OTel logging is enabled, add logrotate config for the RepoHost
+			if err == nil &&
+				postgresCluster.Spec.Instrumentation != nil &&
+				feature.Enabled(ctx, feature.OpenTelemetryLogs) {
+				var pgBackRestLogPath string
+				fmt.Println("shouldn't be here?")
+				for _, repo := range postgresCluster.Spec.Backups.PGBackRest.Repos {
+					if repo.Volume != nil {
+						pgBackRestLogPath = fmt.Sprintf(naming.PGBackRestRepoLogPath, repo.Name)
+						break
+					}
 				}
+				err = collector.AddLogrotateConfig(ctx, postgresCluster.Spec.Instrumentation, cm,
+					pgBackRestLogPath+"/*.log", "")
 			}
-			err = collector.AddLogrotateConfig(ctx, postgresCluster.Spec.Instrumentation, cm,
-				pgBackRestLogPath+"/*.log", "")
 		}
 	}