From 2ba5c87e6ad03e6a46cc429033c7dfaa2c2cf5c7 Mon Sep 17 00:00:00 2001 From: Hongyi Wu Date: Mon, 20 Jul 2026 19:48:17 +0000 Subject: [PATCH] feat: support disabling monitoring for RootSync and RepoSync This commit introduces the ability to opt-out of monitoring sidecars (specifically otel-agent) for RootSync and RepoSync objects via the spec.monitoring.enabled field in the declarative API. When monitoring is disabled, the config-sync controllers will omit the otel-agent container from the reconciler pods and set the environment variable DISABLE_MONITORING=true in the remaining vital containers to silence telemetry exporter logic. All Unit and E2E tests for this logic have been implemented accordingly. TAG=agy --- cmd/hydration-controller/main.go | 16 +- cmd/reconciler-manager/main.go | 64 ++++--- cmd/reconciler/main.go | 16 +- e2e/nomostest/testpredicates/predicates.go | 38 ++++ e2e/testcases/disable_monitoring_test.go | 166 ++++++++++++++++++ manifests/reposync-crd.yaml | 16 ++ manifests/rootsync-crd.yaml | 16 ++ pkg/api/configsync/v1alpha1/reposync_types.go | 4 + pkg/api/configsync/v1alpha1/rootsync_types.go | 4 + pkg/api/configsync/v1alpha1/sync_types.go | 11 ++ .../v1alpha1/zz_generated.conversion.go | 34 ++++ .../v1alpha1/zz_generated.deepcopy.go | 31 ++++ pkg/api/configsync/v1beta1/reposync_types.go | 4 + pkg/api/configsync/v1beta1/rootsync_types.go | 4 + pkg/api/configsync/v1beta1/sync_types.go | 11 ++ .../v1beta1/zz_generated.deepcopy.go | 31 ++++ pkg/kmetrics/register.go | 4 + pkg/metrics/register.go | 5 + pkg/metrics/register_test.go | 54 ++++++ .../controllers/reconciler_base_test.go | 8 +- .../controllers/reposync_controller.go | 19 +- .../controllers/reposync_controller_test.go | 95 ++++++++++ .../controllers/rootsync_controller.go | 19 +- .../controllers/rootsync_controller_test.go | 104 +++++++++++ pkg/reconcilermanager/controllers/util.go | 15 ++ .../controllers/util_test.go | 37 ++++ pkg/reconcilermanager/controllers/volumes.go | 7 +- .../controllers/metrics/register.go | 4 + pkg/resourcegroup/controllers/runner/run.go | 12 +- test/kustomization/expected.yaml | 32 ++++ 30 files changed, 825 insertions(+), 56 deletions(-) create mode 100644 e2e/testcases/disable_monitoring_test.go create mode 100644 pkg/metrics/register_test.go diff --git a/cmd/hydration-controller/main.go b/cmd/hydration-controller/main.go index 21584c13ca..fc07335f55 100644 --- a/cmd/hydration-controller/main.go +++ b/cmd/hydration-controller/main.go @@ -86,13 +86,15 @@ func main() { klog.Fatalf("Failed to register the OTLP metrics exporter: %v", err) } - defer func() { - shutdownCtx, cancel := context.WithTimeout(context.Background(), kmetrics.ShutdownTimeout) - defer cancel() - if err := oce.Shutdown(shutdownCtx); err != nil { - klog.Fatalf("Unable to stop the OTLP metrics exporter: %v", err) - } - }() + if oce != nil { + defer func() { + shutdownCtx, cancel := context.WithTimeout(context.Background(), kmetrics.ShutdownTimeout) + defer cancel() + if err := oce.Shutdown(shutdownCtx); err != nil { + klog.Fatalf("Unable to stop the OTLP metrics exporter: %v", err) + } + }() + } absRepoRootDir, err := cmpath.AbsoluteOS(*repoRootDir) if err != nil { diff --git a/cmd/reconciler-manager/main.go b/cmd/reconciler-manager/main.go index 54b5fa7fa0..6e4797a7a0 100644 --- a/cmd/reconciler-manager/main.go +++ b/cmd/reconciler-manager/main.go @@ -161,26 +161,30 @@ func main() { }) setupLog.Info("RootSync controller registration scheduled") - otelCredentialProvider := &auth.CachingCredentialProvider{ - Scopes: traceapi.DefaultAuthScopes(), - } + if os.Getenv("DISABLE_MONITORING") != "true" { + otelCredentialProvider := &auth.CachingCredentialProvider{ + Scopes: traceapi.DefaultAuthScopes(), + } - otel := controllers.NewOtelReconciler(mgr.GetClient(), - logger.WithName("controllers").WithName("Otel"), - otelCredentialProvider) - if err := otel.Register(mgr); err != nil { - setupLog.Error(err, "failed to register controller", "controller", "Otel") - os.Exit(1) - } - setupLog.Info("Otel controller registration successful") + otel := controllers.NewOtelReconciler(mgr.GetClient(), + logger.WithName("controllers").WithName("Otel"), + otelCredentialProvider) + if err := otel.Register(mgr); err != nil { + setupLog.Error(err, "failed to register controller", "controller", "Otel") + os.Exit(1) + } + setupLog.Info("Otel controller registration successful") - otelSA := controllers.NewOtelSAReconciler(*clusterName, mgr.GetClient(), - logger.WithName("controllers").WithName(controllers.OtelSALoggerName)) - if err := otelSA.Register(mgr); err != nil { - setupLog.Error(err, "failed to register controller", "controller", "OtelSA") - os.Exit(1) + otelSA := controllers.NewOtelSAReconciler(*clusterName, mgr.GetClient(), + logger.WithName("controllers").WithName(controllers.OtelSALoggerName)) + if err := otelSA.Register(mgr); err != nil { + setupLog.Error(err, "failed to register controller", "controller", "OtelSA") + os.Exit(1) + } + setupLog.Info("OtelSA controller registration successful") + } else { + setupLog.Info("Otel and OtelSA controller registration skipped (DISABLE_MONITORING=true)") } - setupLog.Info("OtelSA controller registration successful") // Register the OTLP metrics exporter and metrics instruments ctx := context.Background() @@ -190,13 +194,15 @@ func main() { os.Exit(1) } - defer func() { - shutdownCtx, cancel := context.WithTimeout(context.Background(), metrics.ShutdownTimeout) - defer cancel() - if err := oce.Shutdown(shutdownCtx); err != nil { - setupLog.Error(err, "failed to stop the OTLP metrics exporter") - } - }() + if oce != nil { + defer func() { + shutdownCtx, cancel := context.WithTimeout(context.Background(), metrics.ShutdownTimeout) + defer cancel() + if err := oce.Shutdown(shutdownCtx); err != nil { + setupLog.Error(err, "failed to stop the OTLP metrics exporter") + } + }() + } // +kubebuilder:scaffold:builder @@ -204,10 +210,12 @@ func main() { if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { setupLog.Error(err, "problem running manager") // os.Exit(1) does not run deferred functions so explicitly stopping the OTLP metrics exporter. - shutdownCtx, cancel := context.WithTimeout(context.Background(), metrics.ShutdownTimeout) - defer cancel() - if err := oce.Shutdown(shutdownCtx); err != nil { - setupLog.Error(err, "failed to stop the OTLP metrics exporter") + if oce != nil { + shutdownCtx, cancel := context.WithTimeout(context.Background(), metrics.ShutdownTimeout) + defer cancel() + if err := oce.Shutdown(shutdownCtx); err != nil { + setupLog.Error(err, "failed to stop the OTLP metrics exporter") + } } os.Exit(1) } diff --git a/cmd/reconciler/main.go b/cmd/reconciler/main.go index a6ee675395..85eafeb2a6 100644 --- a/cmd/reconciler/main.go +++ b/cmd/reconciler/main.go @@ -148,13 +148,15 @@ func main() { klog.Fatalf("Failed to register the OTLP metrics exporter: %v", err) } - defer func() { - shutdownCtx, cancel := context.WithTimeout(context.Background(), ocmetrics.ShutdownTimeout) - defer cancel() - if err := oce.Shutdown(shutdownCtx); err != nil { - klog.Fatalf("Unable to stop the OTLP metrics exporter: %v", err) - } - }() + if oce != nil { + defer func() { + shutdownCtx, cancel := context.WithTimeout(context.Background(), ocmetrics.ShutdownTimeout) + defer cancel() + if err := oce.Shutdown(shutdownCtx); err != nil { + klog.Fatalf("Unable to stop the OTLP metrics exporter: %v", err) + } + }() + } absRepoRoot, err := cmpath.AbsoluteOS(*repoRootDir) if err != nil { diff --git a/e2e/nomostest/testpredicates/predicates.go b/e2e/nomostest/testpredicates/predicates.go index e9856dd70f..83d855e09e 100644 --- a/e2e/nomostest/testpredicates/predicates.go +++ b/e2e/nomostest/testpredicates/predicates.go @@ -1669,3 +1669,41 @@ func ConfigMapHasData(key string, value string) Predicate { return fmt.Errorf("%s: %s is not in the ConfigMap", key, value) } } + +// DeploymentMissingVolume returns a predicate that ensures a Deployment does not have a specific volume. +func DeploymentMissingVolume(volumeName string) Predicate { + return func(o client.Object) error { + if o == nil { + return ErrObjectNotFound + } + d, ok := o.(*appsv1.Deployment) + if !ok { + return WrongTypeErr(o, d) + } + for _, volume := range d.Spec.Template.Spec.Volumes { + if volume.Name == volumeName { + return fmt.Errorf("Deployment %s should not have volume %s", core.ObjectNamespacedName(o), volumeName) + } + } + return nil + } +} + +// DeploymentHasVolume returns a predicate that ensures a Deployment has a specific volume. +func DeploymentHasVolume(volumeName string) Predicate { + return func(o client.Object) error { + if o == nil { + return ErrObjectNotFound + } + d, ok := o.(*appsv1.Deployment) + if !ok { + return WrongTypeErr(o, d) + } + for _, volume := range d.Spec.Template.Spec.Volumes { + if volume.Name == volumeName { + return nil + } + } + return fmt.Errorf("Deployment %s should have volume %s", core.ObjectNamespacedName(o), volumeName) + } +} diff --git a/e2e/testcases/disable_monitoring_test.go b/e2e/testcases/disable_monitoring_test.go new file mode 100644 index 0000000000..e48e2f3dab --- /dev/null +++ b/e2e/testcases/disable_monitoring_test.go @@ -0,0 +1,166 @@ +// Copyright 2024 Google LLC +// +// 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 e2e + +import ( + "testing" + + "k8s.io/utils/ptr" + + "github.com/GoogleContainerTools/config-sync/e2e/nomostest" + testmetrics "github.com/GoogleContainerTools/config-sync/e2e/nomostest/metrics" + "github.com/GoogleContainerTools/config-sync/e2e/nomostest/ntopts" + nomostesting "github.com/GoogleContainerTools/config-sync/e2e/nomostest/testing" + "github.com/GoogleContainerTools/config-sync/e2e/nomostest/testpredicates" + "github.com/GoogleContainerTools/config-sync/e2e/nomostest/testwatcher" + "github.com/GoogleContainerTools/config-sync/pkg/api/configsync" + "github.com/GoogleContainerTools/config-sync/pkg/api/configsync/v1beta1" + "github.com/GoogleContainerTools/config-sync/pkg/core" + "github.com/GoogleContainerTools/config-sync/pkg/core/k8sobjects" + "github.com/GoogleContainerTools/config-sync/pkg/kinds" + "github.com/GoogleContainerTools/config-sync/pkg/metrics" + "github.com/GoogleContainerTools/config-sync/pkg/reconcilermanager" +) + +func TestDisableMonitoringRootSync(t *testing.T) { + rootSyncID := nomostest.DefaultRootSyncID + nt := nomostest.New(t, nomostesting.OverrideAPI, + ntopts.SyncWithGitSource(rootSyncID, ntopts.Unstructured)) + + rootReconcilerName := core.RootReconcilerObjectKey(rootSyncID.Name) + rootSyncV1 := k8sobjects.RootSyncObjectV1Beta1(configsync.RootSyncName) + + nt.Must(nt.WatchForAllSyncs()) + + // validate initial monitoring enabled (default state) + nt.Must(nt.Watcher.WatchObject(kinds.Deployment(), + rootReconcilerName.Name, rootReconcilerName.Namespace, + testwatcher.WatchPredicates( + testpredicates.DeploymentHasContainer(metrics.OtelAgentName), + testpredicates.DeploymentHasVolume("otel-agent-config-reconciler-vol"), + testpredicates.DeploymentMissingEnvVar(reconcilermanager.Reconciler, "DISABLE_MONITORING"), + ), + )) + err := nomostest.ValidateStandardMetricsForRootSync(nt, testmetrics.Summary{Sync: rootSyncID.ObjectKey}) + if err != nil { + t.Fatalf("Expected standard metrics to be present when monitoring is enabled, but got: %v", err) + } + + // disable monitoring + nt.MustMergePatch(rootSyncV1, `{"spec": {"monitoring": {"enabled": false}}}`) + + nt.Must(nt.Watcher.WatchObject(kinds.Deployment(), + rootReconcilerName.Name, rootReconcilerName.Namespace, + testwatcher.WatchPredicates( + testpredicates.DeploymentMissingContainer(metrics.OtelAgentName), + testpredicates.DeploymentMissingVolume("otel-agent-config-reconciler-vol"), + testpredicates.DeploymentHasEnvVar(reconcilermanager.Reconciler, "DISABLE_MONITORING", "true"), + ), + )) + nt.Must(nt.WatchForAllSyncs()) + + err = nomostest.ValidateStandardMetricsForRootSync(nt, testmetrics.Summary{Sync: rootSyncID.ObjectKey}) + if err == nil { + t.Fatal("Expected an error when validating metrics for RootSync with disabled monitoring, but got nil") + } + + // re-enable monitoring + nt.MustMergePatch(rootSyncV1, `{"spec": {"monitoring": {"enabled": true}}}`) + + nt.Must(nt.Watcher.WatchObject(kinds.Deployment(), + rootReconcilerName.Name, rootReconcilerName.Namespace, + testwatcher.WatchPredicates( + testpredicates.DeploymentHasContainer(metrics.OtelAgentName), + testpredicates.DeploymentHasVolume("otel-agent-config-reconciler-vol"), + testpredicates.DeploymentMissingEnvVar(reconcilermanager.Reconciler, "DISABLE_MONITORING"), + ), + )) + nt.Must(nt.WatchForAllSyncs()) + + err = nomostest.ValidateStandardMetricsForRootSync(nt, testmetrics.Summary{Sync: rootSyncID.ObjectKey}) + if err != nil { + t.Fatalf("Expected standard metrics to be present after re-enabling monitoring, but got: %v", err) + } +} + +func TestDisableMonitoringRepoSync(t *testing.T) { + repoSyncID := core.RepoSyncID(configsync.RepoSyncName, frontendNamespace) + nt := nomostest.New(t, nomostesting.OverrideAPI, + ntopts.SyncWithGitSource(nomostest.DefaultRootSyncID, ntopts.Unstructured), + ntopts.SyncWithGitSource(repoSyncID)) + rootSyncGitRepo := nt.SyncSourceGitReadWriteRepository(nomostest.DefaultRootSyncID) + repoSyncKey := repoSyncID.ObjectKey + + frontendReconcilerNN := core.NsReconcilerObjectKey(repoSyncID.Namespace, repoSyncID.Name) + repoSyncFrontend := nomostest.RepoSyncObjectV1Beta1FromNonRootRepo(nt, repoSyncKey) + + nt.Must(nt.WatchForAllSyncs()) + + // Verify ns-reconciler-frontend uses the default monitoring state (enabled) + nt.Must(nt.Watcher.WatchObject(kinds.Deployment(), + frontendReconcilerNN.Name, frontendReconcilerNN.Namespace, + testwatcher.WatchPredicates( + testpredicates.DeploymentHasContainer(metrics.OtelAgentName), + testpredicates.DeploymentHasVolume("otel-agent-config-reconciler-vol"), + testpredicates.DeploymentMissingEnvVar(reconcilermanager.Reconciler, "DISABLE_MONITORING"), + ), + )) + err := nomostest.ValidateStandardMetricsForRepoSync(nt, testmetrics.Summary{Sync: repoSyncID.ObjectKey}) + if err != nil { + t.Fatalf("Expected standard metrics to be present when monitoring is enabled, but got: %v", err) + } + + // Disable monitoring + repoSyncFrontend.Spec.Monitoring = &v1beta1.MonitoringSpec{Enabled: ptr.To(false)} + nt.Must(rootSyncGitRepo.Add(nomostest.StructuredNSPath(repoSyncID.Namespace, repoSyncID.Name), repoSyncFrontend)) + nt.Must(rootSyncGitRepo.CommitAndPush("Disable monitoring of frontend Reposync")) + + // validate override and make sure otel-agent is missing + nt.Must(nt.Watcher.WatchObject(kinds.Deployment(), + frontendReconcilerNN.Name, frontendReconcilerNN.Namespace, + testwatcher.WatchPredicates( + testpredicates.DeploymentMissingContainer(metrics.OtelAgentName), + testpredicates.DeploymentMissingVolume("otel-agent-config-reconciler-vol"), + testpredicates.DeploymentHasEnvVar(reconcilermanager.Reconciler, "DISABLE_MONITORING", "true"), + ), + )) + nt.Must(nt.WatchForAllSyncs()) + + err = nomostest.ValidateStandardMetricsForRepoSync(nt, testmetrics.Summary{Sync: repoSyncID.ObjectKey}) + if err == nil { + t.Fatal("Expected an error when validating metrics for RepoSync with disabled monitoring, but got nil") + } + + // Re-enable monitoring + repoSyncFrontend.Spec.Monitoring = &v1beta1.MonitoringSpec{Enabled: ptr.To(true)} + nt.Must(rootSyncGitRepo.Add(nomostest.StructuredNSPath(repoSyncID.Namespace, repoSyncID.Name), repoSyncFrontend)) + nt.Must(rootSyncGitRepo.CommitAndPush("Re-enable monitoring of frontend RepoSync")) + + // validate container comes back + nt.Must(nt.Watcher.WatchObject(kinds.Deployment(), + frontendReconcilerNN.Name, frontendReconcilerNN.Namespace, + testwatcher.WatchPredicates( + testpredicates.DeploymentHasContainer(metrics.OtelAgentName), + testpredicates.DeploymentHasVolume("otel-agent-config-reconciler-vol"), + testpredicates.DeploymentMissingEnvVar(reconcilermanager.Reconciler, "DISABLE_MONITORING"), + ), + )) + nt.Must(nt.WatchForAllSyncs()) + + err = nomostest.ValidateStandardMetricsForRepoSync(nt, testmetrics.Summary{Sync: repoSyncID.ObjectKey}) + if err != nil { + t.Fatalf("Expected standard metrics to be present after re-enabling monitoring, but got: %v", err) + } +} diff --git a/manifests/reposync-crd.yaml b/manifests/reposync-crd.yaml index 563c4664b5..f50c694e99 100644 --- a/manifests/reposync-crd.yaml +++ b/manifests/reposync-crd.yaml @@ -285,6 +285,14 @@ spec: - chart - repo type: object + monitoring: + description: monitoring defines the monitoring configuration. + properties: + enabled: + default: true + description: enabled controls whether metrics exporting is enabled. + type: boolean + type: object oci: description: oci contains configuration specific to importing resources from an OCI package. @@ -1409,6 +1417,14 @@ spec: - chart - repo type: object + monitoring: + description: monitoring defines the monitoring configuration. + properties: + enabled: + default: true + description: enabled controls whether metrics exporting is enabled. + type: boolean + type: object oci: description: oci contains configuration specific to importing resources from an OCI package. diff --git a/manifests/rootsync-crd.yaml b/manifests/rootsync-crd.yaml index f96dcce09f..277484bf52 100644 --- a/manifests/rootsync-crd.yaml +++ b/manifests/rootsync-crd.yaml @@ -297,6 +297,14 @@ spec: - chart - repo type: object + monitoring: + description: monitoring defines the monitoring configuration. + properties: + enabled: + default: true + description: enabled controls whether metrics exporting is enabled. + type: boolean + type: object oci: description: oci contains configuration specific to importing resources from an OCI package. @@ -1482,6 +1490,14 @@ spec: - chart - repo type: object + monitoring: + description: monitoring defines the monitoring configuration. + properties: + enabled: + default: true + description: enabled controls whether metrics exporting is enabled. + type: boolean + type: object oci: description: oci contains configuration specific to importing resources from an OCI package. diff --git a/pkg/api/configsync/v1alpha1/reposync_types.go b/pkg/api/configsync/v1alpha1/reposync_types.go index 36b7d98320..50de7623ee 100644 --- a/pkg/api/configsync/v1alpha1/reposync_types.go +++ b/pkg/api/configsync/v1alpha1/reposync_types.go @@ -80,6 +80,10 @@ type RepoSyncSpec struct { // +nullable // +optional Override *RepoSyncOverrideSpec `json:"override,omitempty"` + + // monitoring specifies the observability configuration for the reconciler. + // +optional + Monitoring *MonitoringSpec `json:"monitoring,omitempty"` } // RepoSyncStatus defines the observed state of a RepoSync. diff --git a/pkg/api/configsync/v1alpha1/rootsync_types.go b/pkg/api/configsync/v1alpha1/rootsync_types.go index db9b0402b9..9e80b97675 100644 --- a/pkg/api/configsync/v1alpha1/rootsync_types.go +++ b/pkg/api/configsync/v1alpha1/rootsync_types.go @@ -80,6 +80,10 @@ type RootSyncSpec struct { // +nullable // +optional Override *RootSyncOverrideSpec `json:"override,omitempty"` + + // monitoring specifies the observability configuration for the reconciler. + // +optional + Monitoring *MonitoringSpec `json:"monitoring,omitempty"` } // RootSyncStatus defines the observed state of RootSync diff --git a/pkg/api/configsync/v1alpha1/sync_types.go b/pkg/api/configsync/v1alpha1/sync_types.go index 450386a796..4f1d8a3afa 100644 --- a/pkg/api/configsync/v1alpha1/sync_types.go +++ b/pkg/api/configsync/v1alpha1/sync_types.go @@ -249,3 +249,14 @@ type ResourceRef struct { // +optional GVK metav1.GroupVersionKind `json:"gvk,omitempty"` } + +// MonitoringSpec controls the inclusion of observational components for the reconciler. +type MonitoringSpec struct { + // enabled controls whether the OpenTelemetry (otel-agent) sidecar is deployed + // alongside the reconciler container. When enabled, the sidecar intercepts + // and exports metrics. + // Default: true. + // +optional + // +kubebuilder:default:=true + Enabled *bool `json:"enabled,omitempty"` +} diff --git a/pkg/api/configsync/v1alpha1/zz_generated.conversion.go b/pkg/api/configsync/v1alpha1/zz_generated.conversion.go index bc9ee479c2..a0fe9a26be 100644 --- a/pkg/api/configsync/v1alpha1/zz_generated.conversion.go +++ b/pkg/api/configsync/v1alpha1/zz_generated.conversion.go @@ -113,6 +113,16 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddGeneratedConversionFunc((*MonitoringSpec)(nil), (*v1beta1.MonitoringSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha1_MonitoringSpec_To_v1beta1_MonitoringSpec(a.(*MonitoringSpec), b.(*v1beta1.MonitoringSpec), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*v1beta1.MonitoringSpec)(nil), (*MonitoringSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_MonitoringSpec_To_v1alpha1_MonitoringSpec(a.(*v1beta1.MonitoringSpec), b.(*MonitoringSpec), scope) + }); err != nil { + return err + } if err := s.AddGeneratedConversionFunc((*Oci)(nil), (*v1beta1.Oci)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha1_Oci_To_v1beta1_Oci(a.(*Oci), b.(*v1beta1.Oci), scope) }); err != nil { @@ -628,6 +638,26 @@ func Convert_v1beta1_HelmStatus_To_v1alpha1_HelmStatus(in *v1beta1.HelmStatus, o return autoConvert_v1beta1_HelmStatus_To_v1alpha1_HelmStatus(in, out, s) } +func autoConvert_v1alpha1_MonitoringSpec_To_v1beta1_MonitoringSpec(in *MonitoringSpec, out *v1beta1.MonitoringSpec, s conversion.Scope) error { + out.Enabled = (*bool)(unsafe.Pointer(in.Enabled)) + return nil +} + +// Convert_v1alpha1_MonitoringSpec_To_v1beta1_MonitoringSpec is an autogenerated conversion function. +func Convert_v1alpha1_MonitoringSpec_To_v1beta1_MonitoringSpec(in *MonitoringSpec, out *v1beta1.MonitoringSpec, s conversion.Scope) error { + return autoConvert_v1alpha1_MonitoringSpec_To_v1beta1_MonitoringSpec(in, out, s) +} + +func autoConvert_v1beta1_MonitoringSpec_To_v1alpha1_MonitoringSpec(in *v1beta1.MonitoringSpec, out *MonitoringSpec, s conversion.Scope) error { + out.Enabled = (*bool)(unsafe.Pointer(in.Enabled)) + return nil +} + +// Convert_v1beta1_MonitoringSpec_To_v1alpha1_MonitoringSpec is an autogenerated conversion function. +func Convert_v1beta1_MonitoringSpec_To_v1alpha1_MonitoringSpec(in *v1beta1.MonitoringSpec, out *MonitoringSpec, s conversion.Scope) error { + return autoConvert_v1beta1_MonitoringSpec_To_v1alpha1_MonitoringSpec(in, out, s) +} + func autoConvert_v1alpha1_Oci_To_v1beta1_Oci(in *Oci, out *v1beta1.Oci, s conversion.Scope) error { out.Image = in.Image out.Dir = in.Dir @@ -899,6 +929,7 @@ func autoConvert_v1alpha1_RepoSyncSpec_To_v1beta1_RepoSyncSpec(in *RepoSyncSpec, out.Helm = nil } out.Override = (*v1beta1.RepoSyncOverrideSpec)(unsafe.Pointer(in.Override)) + out.Monitoring = (*v1beta1.MonitoringSpec)(unsafe.Pointer(in.Monitoring)) return nil } @@ -922,6 +953,7 @@ func autoConvert_v1beta1_RepoSyncSpec_To_v1alpha1_RepoSyncSpec(in *v1beta1.RepoS out.Helm = nil } out.Override = (*RepoSyncOverrideSpec)(unsafe.Pointer(in.Override)) + out.Monitoring = (*MonitoringSpec)(unsafe.Pointer(in.Monitoring)) return nil } @@ -1161,6 +1193,7 @@ func autoConvert_v1alpha1_RootSyncSpec_To_v1beta1_RootSyncSpec(in *RootSyncSpec, out.Helm = nil } out.Override = (*v1beta1.RootSyncOverrideSpec)(unsafe.Pointer(in.Override)) + out.Monitoring = (*v1beta1.MonitoringSpec)(unsafe.Pointer(in.Monitoring)) return nil } @@ -1184,6 +1217,7 @@ func autoConvert_v1beta1_RootSyncSpec_To_v1alpha1_RootSyncSpec(in *v1beta1.RootS out.Helm = nil } out.Override = (*RootSyncOverrideSpec)(unsafe.Pointer(in.Override)) + out.Monitoring = (*MonitoringSpec)(unsafe.Pointer(in.Monitoring)) return nil } diff --git a/pkg/api/configsync/v1alpha1/zz_generated.deepcopy.go b/pkg/api/configsync/v1alpha1/zz_generated.deepcopy.go index b9c4d21a43..0134a6e8a6 100644 --- a/pkg/api/configsync/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/api/configsync/v1alpha1/zz_generated.deepcopy.go @@ -214,6 +214,27 @@ func (in *HelmStatus) DeepCopy() *HelmStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MonitoringSpec) DeepCopyInto(out *MonitoringSpec) { + *out = *in + if in.Enabled != nil { + in, out := &in.Enabled, &out.Enabled + *out = new(bool) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MonitoringSpec. +func (in *MonitoringSpec) DeepCopy() *MonitoringSpec { + if in == nil { + return nil + } + out := new(MonitoringSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Oci) DeepCopyInto(out *Oci) { *out = *in @@ -485,6 +506,11 @@ func (in *RepoSyncSpec) DeepCopyInto(out *RepoSyncSpec) { *out = new(RepoSyncOverrideSpec) (*in).DeepCopyInto(*out) } + if in.Monitoring != nil { + in, out := &in.Monitoring, &out.Monitoring + *out = new(MonitoringSpec) + (*in).DeepCopyInto(*out) + } return } @@ -696,6 +722,11 @@ func (in *RootSyncSpec) DeepCopyInto(out *RootSyncSpec) { *out = new(RootSyncOverrideSpec) (*in).DeepCopyInto(*out) } + if in.Monitoring != nil { + in, out := &in.Monitoring, &out.Monitoring + *out = new(MonitoringSpec) + (*in).DeepCopyInto(*out) + } return } diff --git a/pkg/api/configsync/v1beta1/reposync_types.go b/pkg/api/configsync/v1beta1/reposync_types.go index 29448aa2c0..e06aa0a212 100644 --- a/pkg/api/configsync/v1beta1/reposync_types.go +++ b/pkg/api/configsync/v1beta1/reposync_types.go @@ -81,6 +81,10 @@ type RepoSyncSpec struct { // +nullable // +optional Override *RepoSyncOverrideSpec `json:"override,omitempty"` + + // monitoring specifies the observability configuration for the reconciler. + // +optional + Monitoring *MonitoringSpec `json:"monitoring,omitempty"` } // RepoSyncStatus defines the observed state of a RepoSync. diff --git a/pkg/api/configsync/v1beta1/rootsync_types.go b/pkg/api/configsync/v1beta1/rootsync_types.go index d11c79ccc9..5245c4d5f5 100644 --- a/pkg/api/configsync/v1beta1/rootsync_types.go +++ b/pkg/api/configsync/v1beta1/rootsync_types.go @@ -81,6 +81,10 @@ type RootSyncSpec struct { // +nullable // +optional Override *RootSyncOverrideSpec `json:"override,omitempty"` + + // monitoring specifies the observability configuration for the reconciler. + // +optional + Monitoring *MonitoringSpec `json:"monitoring,omitempty"` } // RootSyncStatus defines the observed state of RootSync diff --git a/pkg/api/configsync/v1beta1/sync_types.go b/pkg/api/configsync/v1beta1/sync_types.go index 2c4c137c6a..770bb2232c 100644 --- a/pkg/api/configsync/v1beta1/sync_types.go +++ b/pkg/api/configsync/v1beta1/sync_types.go @@ -249,3 +249,14 @@ type ResourceRef struct { // +optional GVK metav1.GroupVersionKind `json:"gvk,omitempty"` } + +// MonitoringSpec controls the inclusion of observational components for the reconciler. +type MonitoringSpec struct { + // enabled controls whether the OpenTelemetry (otel-agent) sidecar is deployed + // alongside the reconciler container. When enabled, the sidecar intercepts + // and exports metrics. + // Default: true. + // +optional + // +kubebuilder:default:=true + Enabled *bool `json:"enabled,omitempty"` +} diff --git a/pkg/api/configsync/v1beta1/zz_generated.deepcopy.go b/pkg/api/configsync/v1beta1/zz_generated.deepcopy.go index 87e6c58895..f222247df6 100644 --- a/pkg/api/configsync/v1beta1/zz_generated.deepcopy.go +++ b/pkg/api/configsync/v1beta1/zz_generated.deepcopy.go @@ -214,6 +214,27 @@ func (in *HelmStatus) DeepCopy() *HelmStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MonitoringSpec) DeepCopyInto(out *MonitoringSpec) { + *out = *in + if in.Enabled != nil { + in, out := &in.Enabled, &out.Enabled + *out = new(bool) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MonitoringSpec. +func (in *MonitoringSpec) DeepCopy() *MonitoringSpec { + if in == nil { + return nil + } + out := new(MonitoringSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Oci) DeepCopyInto(out *Oci) { *out = *in @@ -485,6 +506,11 @@ func (in *RepoSyncSpec) DeepCopyInto(out *RepoSyncSpec) { *out = new(RepoSyncOverrideSpec) (*in).DeepCopyInto(*out) } + if in.Monitoring != nil { + in, out := &in.Monitoring, &out.Monitoring + *out = new(MonitoringSpec) + (*in).DeepCopyInto(*out) + } return } @@ -696,6 +722,11 @@ func (in *RootSyncSpec) DeepCopyInto(out *RootSyncSpec) { *out = new(RootSyncOverrideSpec) (*in).DeepCopyInto(*out) } + if in.Monitoring != nil { + in, out := &in.Monitoring, &out.Monitoring + *out = new(MonitoringSpec) + (*in).DeepCopyInto(*out) + } return } diff --git a/pkg/kmetrics/register.go b/pkg/kmetrics/register.go index e9504fcb4b..e0433479c8 100644 --- a/pkg/kmetrics/register.go +++ b/pkg/kmetrics/register.go @@ -32,6 +32,10 @@ const ( // RegisterOTelExporter creates the OTLP metrics exporter. func RegisterOTelExporter(ctx context.Context, containerName string) (*otlpmetricgrpc.Exporter, error) { + if os.Getenv("DISABLE_MONITORING") == "true" { + err := InitializeOTelKustomizeMetrics() + return nil, err + } err := os.Setenv( "OTEL_RESOURCE_ATTRIBUTES", diff --git a/pkg/metrics/register.go b/pkg/metrics/register.go index ad99cf65d7..29c973fda4 100644 --- a/pkg/metrics/register.go +++ b/pkg/metrics/register.go @@ -33,6 +33,11 @@ const ( // RegisterOTelExporter creates the OTLP metrics exporter. func RegisterOTelExporter(ctx context.Context, containerName string) (*otlpmetricgrpc.Exporter, error) { + if os.Getenv("DISABLE_MONITORING") == "true" { + klog.V(5).Infof("METRIC DEBUG: Monitoring is disabled via DISABLE_MONITORING env var") + err := InitializeOTelMetrics() + return nil, err + } klog.V(5).Infof("METRIC DEBUG: Registering OTLP exporter for container: %q", containerName) err := os.Setenv( diff --git a/pkg/metrics/register_test.go b/pkg/metrics/register_test.go new file mode 100644 index 0000000000..5320c3f676 --- /dev/null +++ b/pkg/metrics/register_test.go @@ -0,0 +1,54 @@ +// Copyright 2024 Google LLC +// +// 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. + +package metrics + +import ( + "context" + "os" + "testing" +) + +func TestRegisterOTelExporter_DisableMonitoring(t *testing.T) { + // Save the original value and defer restoration + original := os.Getenv("DISABLE_MONITORING") + defer func() { + if err := os.Setenv("DISABLE_MONITORING", original); err != nil { + t.Errorf("failed to restore DISABLE_MONITORING env var: %v", err) + } + }() + + err := os.Setenv("DISABLE_MONITORING", "true") + if err != nil { + t.Fatalf("failed to set DISABLE_MONITORING env var: %v", err) + } + + ctx := context.Background() + exporter, err := RegisterOTelExporter(ctx, "test-container") + if err != nil { + t.Fatalf("expected no error when monitoring is disabled, got %v", err) + } + + if exporter != nil { + t.Fatalf("expected exporter to be nil when monitoring is disabled") + } + + // In the real code we wrap exporter.Shutdown in 'if exporter != nil' to prevent panics. + // We simulate that fix here to ensure we don't panic on Shutdown. + func() { + if exporter != nil { + if err := exporter.Shutdown(ctx); err != nil { + t.Errorf("expected no error on shutdown") + } + } + }() +} diff --git a/pkg/reconcilermanager/controllers/reconciler_base_test.go b/pkg/reconcilermanager/controllers/reconciler_base_test.go index 30847128ed..d1e4e9f96c 100644 --- a/pkg/reconcilermanager/controllers/reconciler_base_test.go +++ b/pkg/reconcilermanager/controllers/reconciler_base_test.go @@ -396,7 +396,7 @@ spec: - containerPort: 8888 # Metrics. protocol: TCP volumeMounts: - - name: otel-agent-config-vol + - name: otel-agent-config-reconciler-vol mountPath: /conf livenessProbe: httpGet: @@ -476,7 +476,7 @@ spec: - containerPort: 8888 # Metrics. protocol: TCP volumeMounts: - - name: otel-agent-config-vol + - name: otel-agent-config-reconciler-vol mountPath: /conf livenessProbe: httpGet: @@ -548,7 +548,7 @@ func TestCompareDeploymentsToCreatePatchData(t *testing.T) { current: currentDeploymentUnstructured, isAutopilot: false, expectedSame: false, - expectedPatch: `{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"labels":{"app":"reconciler","configmanagement.gke.io/arch":"csmr","configmanagement.gke.io/system":"true","test-data":"true"},"namespace":"config-management-system"},"spec":{"minReadySeconds":10,"replicas":1,"selector":{"matchLabels":{"app":"reconciler","configsync.gke.io/deployment-name":""}},"strategy":{"type":"Recreate"},"template":{"metadata":{},"spec":{"containers":[{"args":["--config=/conf/otel-agent-config.yaml","--feature-gates=-exporter.googlecloud.OTLPDirect"],"command":["/otelcol-contrib"],"image":"gcr.io/config-management-release/otelcontribcol:v0.54.0","imagePullPolicy":"IfNotPresent","livenessProbe":{"httpGet":{"path":"/","port":13133,"scheme":"HTTP"}},"name":"otel-agent","ports":[{"containerPort":4317,"protocol":"TCP"},{"containerPort":8888,"protocol":"TCP"}],"readinessProbe":{"httpGet":{"path":"/","port":13133,"scheme":"HTTP"}},"resources":{"requests":{"cpu":"10m","memory":"100Mi"}},"securityContext":{"allowPrivilegeEscalation":false,"capabilities":{"drop":["NET_RAW"]},"readOnlyRootFilesystem":true},"volumeMounts":[{"mountPath":"/conf","name":"otel-agent-config-vol"}]}],"dnsPolicy":"ClusterFirst","schedulerName":"default-scheduler","securityContext":{"fsGroup":65533,"runAsNonRoot":true,"runAsUser":1000,"seccompProfile":{"type":"RuntimeDefault"}},"tolerations":[{"key":"foo"}],"volumes":[{"emptyDir":{},"name":"repo"},{"emptyDir":{},"name":"kube"}]}}},"status":{}}`, + expectedPatch: `{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"labels":{"app":"reconciler","configmanagement.gke.io/arch":"csmr","configmanagement.gke.io/system":"true","test-data":"true"},"namespace":"config-management-system"},"spec":{"minReadySeconds":10,"replicas":1,"selector":{"matchLabels":{"app":"reconciler","configsync.gke.io/deployment-name":""}},"strategy":{"type":"Recreate"},"template":{"metadata":{},"spec":{"containers":[{"args":["--config=/conf/otel-agent-config.yaml","--feature-gates=-exporter.googlecloud.OTLPDirect"],"command":["/otelcol-contrib"],"image":"gcr.io/config-management-release/otelcontribcol:v0.54.0","imagePullPolicy":"IfNotPresent","livenessProbe":{"httpGet":{"path":"/","port":13133,"scheme":"HTTP"}},"name":"otel-agent","ports":[{"containerPort":4317,"protocol":"TCP"},{"containerPort":8888,"protocol":"TCP"}],"readinessProbe":{"httpGet":{"path":"/","port":13133,"scheme":"HTTP"}},"resources":{"requests":{"cpu":"10m","memory":"100Mi"}},"securityContext":{"allowPrivilegeEscalation":false,"capabilities":{"drop":["NET_RAW"]},"readOnlyRootFilesystem":true},"volumeMounts":[{"mountPath":"/conf","name":"otel-agent-config-reconciler-vol"}]}],"dnsPolicy":"ClusterFirst","schedulerName":"default-scheduler","securityContext":{"fsGroup":65533,"runAsNonRoot":true,"runAsUser":1000,"seccompProfile":{"type":"RuntimeDefault"}},"tolerations":[{"key":"foo"}],"volumes":[{"emptyDir":{},"name":"repo"},{"emptyDir":{},"name":"kube"}]}}},"status":{}}`, }, "Deployment should preserve custom tolerations for Autopilot": { declared: func() *appsv1.Deployment { @@ -570,7 +570,7 @@ func TestCompareDeploymentsToCreatePatchData(t *testing.T) { current: currentDeploymentUnstructured, isAutopilot: true, expectedSame: false, - expectedPatch: `{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"labels":{"app":"reconciler","configmanagement.gke.io/arch":"csmr","configmanagement.gke.io/system":"true","test-data":"true"},"namespace":"config-management-system"},"spec":{"minReadySeconds":10,"replicas":1,"selector":{"matchLabels":{"app":"reconciler","configsync.gke.io/deployment-name":""}},"strategy":{"type":"Recreate"},"template":{"metadata":{},"spec":{"containers":[{"args":["--config=/conf/otel-agent-config.yaml","--feature-gates=-exporter.googlecloud.OTLPDirect"],"command":["/otelcol-contrib"],"image":"gcr.io/config-management-release/otelcontribcol:v0.54.0","imagePullPolicy":"IfNotPresent","livenessProbe":{"httpGet":{"path":"/","port":13133,"scheme":"HTTP"}},"name":"otel-agent","ports":[{"containerPort":4317,"protocol":"TCP"},{"containerPort":8888,"protocol":"TCP"}],"readinessProbe":{"httpGet":{"path":"/","port":13133,"scheme":"HTTP"}},"resources":{"requests":{"cpu":"10m","memory":"100Mi"}},"securityContext":{"allowPrivilegeEscalation":false,"capabilities":{"drop":["NET_RAW"]},"readOnlyRootFilesystem":true},"volumeMounts":[{"mountPath":"/conf","name":"otel-agent-config-vol"}]}],"dnsPolicy":"ClusterFirst","schedulerName":"default-scheduler","securityContext":{"fsGroup":65533,"runAsNonRoot":true,"runAsUser":1000,"seccompProfile":{"type":"RuntimeDefault"}},"tolerations":[{"key":"foo"}],"volumes":[{"emptyDir":{},"name":"repo"},{"emptyDir":{},"name":"kube"}]}}},"status":{}}`, + expectedPatch: `{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"labels":{"app":"reconciler","configmanagement.gke.io/arch":"csmr","configmanagement.gke.io/system":"true","test-data":"true"},"namespace":"config-management-system"},"spec":{"minReadySeconds":10,"replicas":1,"selector":{"matchLabels":{"app":"reconciler","configsync.gke.io/deployment-name":""}},"strategy":{"type":"Recreate"},"template":{"metadata":{},"spec":{"containers":[{"args":["--config=/conf/otel-agent-config.yaml","--feature-gates=-exporter.googlecloud.OTLPDirect"],"command":["/otelcol-contrib"],"image":"gcr.io/config-management-release/otelcontribcol:v0.54.0","imagePullPolicy":"IfNotPresent","livenessProbe":{"httpGet":{"path":"/","port":13133,"scheme":"HTTP"}},"name":"otel-agent","ports":[{"containerPort":4317,"protocol":"TCP"},{"containerPort":8888,"protocol":"TCP"}],"readinessProbe":{"httpGet":{"path":"/","port":13133,"scheme":"HTTP"}},"resources":{"requests":{"cpu":"10m","memory":"100Mi"}},"securityContext":{"allowPrivilegeEscalation":false,"capabilities":{"drop":["NET_RAW"]},"readOnlyRootFilesystem":true},"volumeMounts":[{"mountPath":"/conf","name":"otel-agent-config-reconciler-vol"}]}],"dnsPolicy":"ClusterFirst","schedulerName":"default-scheduler","securityContext":{"fsGroup":65533,"runAsNonRoot":true,"runAsUser":1000,"seccompProfile":{"type":"RuntimeDefault"}},"tolerations":[{"key":"foo"}],"volumes":[{"emptyDir":{},"name":"repo"},{"emptyDir":{},"name":"kube"}]}}},"status":{}}`, }, } diff --git a/pkg/reconcilermanager/controllers/reposync_controller.go b/pkg/reconcilermanager/controllers/reposync_controller.go index b970d2d917..226d3f5462 100644 --- a/pkg/reconcilermanager/controllers/reposync_controller.go +++ b/pkg/reconcilermanager/controllers/reposync_controller.go @@ -18,6 +18,7 @@ import ( "context" "errors" "fmt" + "slices" "strings" "sync" @@ -971,6 +972,16 @@ func (r *RepoSyncReconciler) populateContainerEnvs(ctx context.Context, rs *v1be caCertSecretRef: v1beta1.GetSecretName(rs.Spec.Helm.CACertSecretRef), }) } + + if !IsMonitoringEnabled(rs.Spec.Monitoring) { + for containerName, envs := range result { + result[containerName] = append(envs, corev1.EnvVar{ + Name: "DISABLE_MONITORING", + Value: "true", + }) + } + } + return result, nil } @@ -1201,7 +1212,7 @@ func (r *RepoSyncReconciler) mutationsFor(ctx context.Context, rs *v1beta1.RepoS if useCACert(caCertSecretRefName) { caCertSecretRefName = ReconcilerResourceName(reconcilerName, caCertSecretRefName) } - templateSpec.Volumes = filterVolumes(templateSpec.Volumes, auth, secretName, caCertSecretRefName, rs.Spec.SourceType, r.membership) + templateSpec.Volumes = filterVolumes(templateSpec.Volumes, auth, secretName, caCertSecretRefName, rs.Spec.SourceType, r.membership, rs.Spec.Monitoring) autopilot, err := r.isAutopilot() if err != nil { @@ -1288,7 +1299,11 @@ func (r *RepoSyncReconciler) mutationsFor(ctx context.Context, rs *v1beta1.RepoS // TODO: enable resource/logLevel overrides for gcenode-askpass-sidecar } case metrics.OtelAgentName: - container.Env = append(container.Env, containerEnvs[container.Name]...) + if !IsMonitoringEnabled(rs.Spec.Monitoring) { + addContainer = false + } else { + container.Env = append(container.Env, containerEnvs[container.Name]...) + } default: return fmt.Errorf("unknown container in reconciler deployment template: %q", container.Name) } diff --git a/pkg/reconcilermanager/controllers/reposync_controller_test.go b/pkg/reconcilermanager/controllers/reposync_controller_test.go index b640d09ee1..617492a3b1 100644 --- a/pkg/reconcilermanager/controllers/reposync_controller_test.go +++ b/pkg/reconcilermanager/controllers/reposync_controller_test.go @@ -34,6 +34,7 @@ import ( "github.com/GoogleContainerTools/config-sync/pkg/core/k8sobjects" "github.com/GoogleContainerTools/config-sync/pkg/kinds" "github.com/GoogleContainerTools/config-sync/pkg/metadata" + "github.com/GoogleContainerTools/config-sync/pkg/metrics" "github.com/GoogleContainerTools/config-sync/pkg/reconcilermanager" "github.com/GoogleContainerTools/config-sync/pkg/reposync" syncerFake "github.com/GoogleContainerTools/config-sync/pkg/syncer/syncertest/fake" @@ -5368,3 +5369,97 @@ func newDeploymentCondition(condType appsv1.DeploymentConditionType, status core Message: message, } } + +func TestRepoSyncReconcile_DisableMonitoring(t *testing.T) { + // Mock out parseDeployment for testing + parseDeployment = func(de *appsv1.Deployment) error { + de.Spec = appsv1.DeploymentSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + metadata.ReconcilerLabel: reconcilermanager.Reconciler, + }, + }, + Replicas: &reconcilerDeploymentReplicaCount, + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: append(defaultContainers(), corev1.Container{ + Name: "otel-agent", + Image: "otel-agent-image", + }), + Volumes: []corev1.Volume{{Name: "repo"}, {Name: metrics.OtelAgentName + "-config-reconciler-vol"}}, + }, + }, + } + return nil + } + + t.Run("Monitoring.Enabled=false", func(t *testing.T) { + f := false + rs := repoSyncWithGit(reposyncName, reposyncNs, reposyncRef(gitRevision), reposyncBranch(branch), reposyncSecretType(GitSecretConfigKeySSH), reposyncSecretRef(reposyncSSHKey)) + rs.Spec.Monitoring = &v1beta1.MonitoringSpec{Enabled: &f} + + reqNamespacedName := namespacedName(rs.Name, rs.Namespace) + fakeClient, _, testReconciler := setupNSReconciler(t, rs, secretObj(t, reposyncSSHKey, configsync.AuthSSH, configsync.GitSource, core.Namespace(rs.Namespace))) + + ctx := t.Context() + if _, err := testReconciler.Reconcile(ctx, reqNamespacedName); err != nil { + t.Fatalf("unexpected reconciliation error, got error: %q, want error: nil", err) + } + + // Verify deployment + dep := &appsv1.Deployment{} + err := fakeClient.Get(ctx, client.ObjectKey{ + Name: core.NsReconcilerName(rs.Namespace, rs.Name), + Namespace: configsync.ControllerNamespace, + }, dep) + require.NoError(t, err) + + // Verify otel-agent is NOT present + for _, container := range dep.Spec.Template.Spec.Containers { + require.NotEqual(t, "otel-agent", container.Name, "otel-agent should be omitted") + } + + // Verify DISABLE_MONITORING=true is in reconciler and hydration-controller + for _, container := range dep.Spec.Template.Spec.Containers { + if container.Name == reconcilermanager.Reconciler || container.Name == reconcilermanager.HydrationController { + hasEnv := false + for _, env := range container.Env { + if env.Name == "DISABLE_MONITORING" { + require.Equal(t, "true", env.Value) + hasEnv = true + } + } + require.True(t, hasEnv, "DISABLE_MONITORING env var should be present in %s", container.Name) + } + } + }) + + t.Run("Monitoring.Enabled=true (default)", func(t *testing.T) { + rs := repoSyncWithGit(reposyncName, reposyncNs, reposyncRef(gitRevision), reposyncBranch(branch), reposyncSecretType(GitSecretConfigKeySSH), reposyncSecretRef(reposyncSSHKey)) + + reqNamespacedName := namespacedName(rs.Name, rs.Namespace) + fakeClient, _, testReconciler := setupNSReconciler(t, rs, secretObj(t, reposyncSSHKey, configsync.AuthSSH, configsync.GitSource, core.Namespace(rs.Namespace))) + + ctx := t.Context() + if _, err := testReconciler.Reconcile(ctx, reqNamespacedName); err != nil { + t.Fatalf("unexpected reconciliation error, got error: %q, want error: nil", err) + } + + // Verify deployment + dep := &appsv1.Deployment{} + err := fakeClient.Get(ctx, client.ObjectKey{ + Name: core.NsReconcilerName(rs.Namespace, rs.Name), + Namespace: configsync.ControllerNamespace, + }, dep) + require.NoError(t, err) + + // Verify otel-agent IS present + hasOtel := false + for _, container := range dep.Spec.Template.Spec.Containers { + if container.Name == "otel-agent" { + hasOtel = true + } + } + require.True(t, hasOtel, "otel-agent should be present") + }) +} diff --git a/pkg/reconcilermanager/controllers/rootsync_controller.go b/pkg/reconcilermanager/controllers/rootsync_controller.go index 9ad9f4dcb9..7431958907 100644 --- a/pkg/reconcilermanager/controllers/rootsync_controller.go +++ b/pkg/reconcilermanager/controllers/rootsync_controller.go @@ -18,6 +18,7 @@ import ( "context" "errors" "fmt" + "slices" "strings" "sync" @@ -832,6 +833,16 @@ func (r *RootSyncReconciler) populateContainerEnvs(ctx context.Context, rs *v1be caCertSecretRef: v1beta1.GetSecretName(rs.Spec.Helm.CACertSecretRef), }) } + + if !IsMonitoringEnabled(rs.Spec.Monitoring) { + for containerName, envs := range result { + result[containerName] = append(envs, corev1.EnvVar{ + Name: "DISABLE_MONITORING", + Value: "true", + }) + } + } + return result, nil } @@ -1164,7 +1175,7 @@ func (r *RootSyncReconciler) mutationsFor(ctx context.Context, rs *v1beta1.RootS // Secret reference is the name of the secret used by git-sync or helm-sync container to // authenticate with the git or helm repository using the authorization method specified // in the RootSync CR. - templateSpec.Volumes = filterVolumes(templateSpec.Volumes, auth, secretRefName, caCertSecretRefName, rs.Spec.SourceType, r.membership) + templateSpec.Volumes = filterVolumes(templateSpec.Volumes, auth, secretRefName, caCertSecretRefName, rs.Spec.SourceType, r.membership, rs.Spec.Monitoring) autopilot, err := r.isAutopilot() if err != nil { @@ -1251,7 +1262,11 @@ func (r *RootSyncReconciler) mutationsFor(ctx context.Context, rs *v1beta1.RootS // TODO: enable resource/logLevel overrides for gcenode-askpass-sidecar } case metrics.OtelAgentName: - container.Env = append(container.Env, containerEnvs[container.Name]...) + if !IsMonitoringEnabled(rs.Spec.Monitoring) { + addContainer = false + } else { + container.Env = append(container.Env, containerEnvs[container.Name]...) + } default: return fmt.Errorf("unknown container in reconciler deployment template: %q", container.Name) } diff --git a/pkg/reconcilermanager/controllers/rootsync_controller_test.go b/pkg/reconcilermanager/controllers/rootsync_controller_test.go index 3b23c6b987..dc0cfa8342 100644 --- a/pkg/reconcilermanager/controllers/rootsync_controller_test.go +++ b/pkg/reconcilermanager/controllers/rootsync_controller_test.go @@ -5287,3 +5287,107 @@ func helmDeploymentSecretVolumes(secretName string) []corev1.Volume { } return volumes } + +func TestReconcile_DisableMonitoring(t *testing.T) { + // Mock out parseDeployment for testing. + // We use a template that INCLUDES otel-agent. + parseDeployment = func(de *appsv1.Deployment) error { + de.Spec = appsv1.DeploymentSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + metadata.ReconcilerLabel: reconcilermanager.Reconciler, + }, + }, + Replicas: &reconcilerDeploymentReplicaCount, + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: append(defaultContainers(), corev1.Container{ + Name: "otel-agent", + Image: "otel-agent-image", + }), + Volumes: deploymentSecretVolumes(rootsyncSSHKey, ""), + }, + }, + } + return nil + } + + t.Run("Monitoring.Enabled=false", func(t *testing.T) { + f := false + + rs := rootSyncWithGit(rootsyncName, rootsyncRef(gitRevision), rootsyncBranch(branch), rootsyncSecretType(GitSecretConfigKeySSH), rootsyncSecretRef(rootsyncSSHKey)) + rs.Spec.Monitoring = &v1beta1.MonitoringSpec{Enabled: &f} + reqNamespacedName := namespacedName(rs.Name, rs.Namespace) + fakeClient, _, testReconciler := setupRootReconciler(t, rs, secretObj(t, rootsyncSSHKey, configsync.AuthSSH, configsync.GitSource, core.Namespace(rs.Namespace))) + + ctx := t.Context() + if _, err := testReconciler.Reconcile(ctx, reqNamespacedName); err != nil { + t.Fatalf("unexpected reconciliation error, got error: %q, want error: nil", err) + } + + // Verify deployment + dep := &appsv1.Deployment{} + err := fakeClient.Get(ctx, client.ObjectKey{ + Name: rootReconcilerName, + Namespace: configsync.ControllerNamespace, + }, dep) + require.NoError(t, err) + + // Verify otel-agent is NOT present + for _, container := range dep.Spec.Template.Spec.Containers { + require.NotEqual(t, "otel-agent", container.Name, "otel-agent should be omitted") + } + + // Verify DISABLE_MONITORING=true is in reconciler and hydration-controller + for _, container := range dep.Spec.Template.Spec.Containers { + if container.Name == reconcilermanager.Reconciler || container.Name == reconcilermanager.HydrationController { + hasEnv := false + for _, env := range container.Env { + if env.Name == "DISABLE_MONITORING" { + require.Equal(t, "true", env.Value) + hasEnv = true + } + } + require.True(t, hasEnv, "DISABLE_MONITORING env var should be present in %s", container.Name) + } + } + }) + + t.Run("Monitoring.Enabled=true (default)", func(t *testing.T) { + + rs := rootSyncWithGit(rootsyncName, rootsyncRef(gitRevision), rootsyncBranch(branch), rootsyncSecretType(GitSecretConfigKeySSH), rootsyncSecretRef(rootsyncSSHKey)) + reqNamespacedName := namespacedName(rs.Name, rs.Namespace) + fakeClient, _, testReconciler := setupRootReconciler(t, rs, secretObj(t, rootsyncSSHKey, configsync.AuthSSH, configsync.GitSource, core.Namespace(rs.Namespace))) + + ctx := t.Context() + if _, err := testReconciler.Reconcile(ctx, reqNamespacedName); err != nil { + t.Fatalf("unexpected reconciliation error, got error: %q, want error: nil", err) + } + + // Verify deployment + dep := &appsv1.Deployment{} + err := fakeClient.Get(ctx, client.ObjectKey{ + Name: rootReconcilerName, + Namespace: configsync.ControllerNamespace, + }, dep) + require.NoError(t, err) + + // Verify otel-agent IS present + hasOtel := false + for _, container := range dep.Spec.Template.Spec.Containers { + if container.Name == "otel-agent" { + hasOtel = true + } + } + require.True(t, hasOtel, "otel-agent should be present") + + // Verify DISABLE_MONITORING is NOT in reconciler + for _, container := range dep.Spec.Template.Spec.Containers { + if container.Name == reconcilermanager.Reconciler { + for _, env := range container.Env { + require.NotEqual(t, "DISABLE_MONITORING", env.Name, "DISABLE_MONITORING env var should not be present") + } + } + } + }) +} diff --git a/pkg/reconcilermanager/controllers/util.go b/pkg/reconcilermanager/controllers/util.go index 10f91c1cc1..c03d059d45 100644 --- a/pkg/reconcilermanager/controllers/util.go +++ b/pkg/reconcilermanager/controllers/util.go @@ -41,6 +41,21 @@ func updateHydrationControllerImage(image string, overrides v1beta1.OverrideSpec return strings.ReplaceAll(image, reconcilermanager.HydrationController+":", reconcilermanager.HydrationControllerWithShell+":") } +// IsMonitoringEnabled returns true if monitoring is enabled. +// It defaults to true if the spec or the enabled field is nil. +// +// Note on naming: The CRD API uses positive naming (spec.monitoring.enabled) +// to follow Kubernetes API conventions. Internally, this translates to the +// negative environment variable DISABLE_MONITORING=true. This negative opt-out +// naming is used so that when the environment variable is unset, monitoring +// defaults to being enabled, preserving backward compatibility. +func IsMonitoringEnabled(spec *v1beta1.MonitoringSpec) bool { + if spec == nil || spec.Enabled == nil { + return true + } + return *spec.Enabled +} + type hydrationOptions struct { sourceType configsync.SourceType gitConfig *v1beta1.Git diff --git a/pkg/reconcilermanager/controllers/util_test.go b/pkg/reconcilermanager/controllers/util_test.go index 4bfd4aaf88..ffaebafcf4 100644 --- a/pkg/reconcilermanager/controllers/util_test.go +++ b/pkg/reconcilermanager/controllers/util_test.go @@ -221,3 +221,40 @@ func TestOCISyncEnvs(t *testing.T) { }) } } + +func TestIsMonitoringEnabled(t *testing.T) { + trueVal := true + falseVal := false + testCases := []struct { + name string + spec *v1beta1.MonitoringSpec + expected bool + }{ + { + name: "nil spec", + spec: nil, + expected: true, + }, + { + name: "empty spec", + spec: &v1beta1.MonitoringSpec{}, + expected: true, + }, + { + name: "spec with enabled true", + spec: &v1beta1.MonitoringSpec{Enabled: &trueVal}, + expected: true, + }, + { + name: "spec with enabled false", + spec: &v1beta1.MonitoringSpec{Enabled: &falseVal}, + expected: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := IsMonitoringEnabled(tc.spec) + assert.Equal(t, tc.expected, result) + }) + } +} diff --git a/pkg/reconcilermanager/controllers/volumes.go b/pkg/reconcilermanager/controllers/volumes.go index 34c38355f6..95e67311e5 100644 --- a/pkg/reconcilermanager/controllers/volumes.go +++ b/pkg/reconcilermanager/controllers/volumes.go @@ -20,8 +20,10 @@ import ( "time" "github.com/GoogleContainerTools/config-sync/pkg/api/configsync" + "github.com/GoogleContainerTools/config-sync/pkg/api/configsync/v1beta1" hubv1 "github.com/GoogleContainerTools/config-sync/pkg/api/hub/v1" "github.com/GoogleContainerTools/config-sync/pkg/metadata" + "github.com/GoogleContainerTools/config-sync/pkg/metrics" corev1 "k8s.io/api/core/v1" ) @@ -51,10 +53,13 @@ var expirationSeconds = int64((48 * time.Hour).Seconds()) // filterVolumes returns the volumes depending on different auth types. // If authType is `none`, `gcenode`, or `gcpserviceaccount`, it won't mount the `git-creds` volume. // If authType is `gcpserviceaccount` with fleet membership available, it also mounts a `gcp-ksa` volume. -func filterVolumes(existing []corev1.Volume, authType configsync.AuthType, secretName, caCertSecretName string, sourceType configsync.SourceType, membership *hubv1.Membership) []corev1.Volume { +func filterVolumes(existing []corev1.Volume, authType configsync.AuthType, secretName, caCertSecretName string, sourceType configsync.SourceType, membership *hubv1.Membership, monitoring *v1beta1.MonitoringSpec) []corev1.Volume { var updatedVolumes []corev1.Volume for _, volume := range existing { + if volume.Name == metrics.OtelAgentName+"-config-reconciler-vol" && !IsMonitoringEnabled(monitoring) { + continue + } if volume.Name == GitCredentialVolume { // Don't mount git-creds volume if auth is 'none', 'gcenode', or 'gcpserviceaccount' if SkipForAuth(authType) || sourceType != configsync.GitSource { diff --git a/pkg/resourcegroup/controllers/metrics/register.go b/pkg/resourcegroup/controllers/metrics/register.go index 974a337021..cd124cbd9e 100644 --- a/pkg/resourcegroup/controllers/metrics/register.go +++ b/pkg/resourcegroup/controllers/metrics/register.go @@ -26,6 +26,10 @@ import ( // RegisterOTelExporter creates the OTLP metrics exporter. func RegisterOTelExporter(ctx context.Context, containerName string) (*otlpmetricgrpc.Exporter, error) { + if os.Getenv("DISABLE_MONITORING") == "true" { + err := InitializeOTelResourceGroupMetrics() + return nil, err + } err := os.Setenv( "OTEL_RESOURCE_ATTRIBUTES", diff --git a/pkg/resourcegroup/controllers/runner/run.go b/pkg/resourcegroup/controllers/runner/run.go index f5f9ba7b9f..23e17f0dd7 100644 --- a/pkg/resourcegroup/controllers/runner/run.go +++ b/pkg/resourcegroup/controllers/runner/run.go @@ -81,11 +81,13 @@ func run() error { return fmt.Errorf("failed to register the OTLP metrics exporter: %w", err) } - defer func() { - if err := oce.Shutdown(ctx); err != nil { - klog.Error(err, "Unable to stop the OTLP metrics exporter") - } - }() + if oce != nil { + defer func() { + if err := oce.Shutdown(ctx); err != nil { + klog.Error(err, "Unable to stop the OTLP metrics exporter") + } + }() + } mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Logger: logger.WithName("controller-manager"), Scheme: scheme, diff --git a/test/kustomization/expected.yaml b/test/kustomization/expected.yaml index 6d7bdb91ec..1f63e71216 100644 --- a/test/kustomization/expected.yaml +++ b/test/kustomization/expected.yaml @@ -680,6 +680,14 @@ spec: - chart - repo type: object + monitoring: + description: monitoring defines the monitoring configuration. + properties: + enabled: + default: true + description: enabled controls whether metrics exporting is enabled. + type: boolean + type: object oci: description: oci contains configuration specific to importing resources from an OCI package. @@ -1804,6 +1812,14 @@ spec: - chart - repo type: object + monitoring: + description: monitoring defines the monitoring configuration. + properties: + enabled: + default: true + description: enabled controls whether metrics exporting is enabled. + type: boolean + type: object oci: description: oci contains configuration specific to importing resources from an OCI package. @@ -3253,6 +3269,14 @@ spec: - chart - repo type: object + monitoring: + description: monitoring defines the monitoring configuration. + properties: + enabled: + default: true + description: enabled controls whether metrics exporting is enabled. + type: boolean + type: object oci: description: oci contains configuration specific to importing resources from an OCI package. @@ -4438,6 +4462,14 @@ spec: - chart - repo type: object + monitoring: + description: monitoring defines the monitoring configuration. + properties: + enabled: + default: true + description: enabled controls whether metrics exporting is enabled. + type: boolean + type: object oci: description: oci contains configuration specific to importing resources from an OCI package.