prometheusScraping: Remove prometheus scraping flag

Signed-off-by: Sneha Chhabria <[email protected]>

.env.example

@@ -106,10 +106,6 @@ export BOOKWAREHOUSE_NAMESPACE=bookwarehouse
 # Default: false
 # export DEPLOY_PROMETHEUS=true
 
-# optional: ENABLE_PROMETHEUS_SCRAPING (true/false)
-# Default: true
-# export ENABLE_PROMETHEUS_SCRAPING=true
-
 # optional: Maximum of iterations to test for expected return codes. 0 means unlimited.
 # export CI_MAX_ITERATIONS_THRESHOLD=0
 

charts/osm/README.md

@@ -75,7 +75,6 @@ The following table lists the configurable parameters of the osm chart and their
 | OpenServiceMesh.enableFluentbit | bool | `false` | Enable Fluent Bit sidecar deployment on OSM controller's pod |
 | OpenServiceMesh.enablePermissiveTrafficPolicy | bool | `false` | Enable permissive traffic policy mode |
 | OpenServiceMesh.enablePrivilegedInitContainer | bool | `false` | Run init container in privileged mode |
-| OpenServiceMesh.enablePrometheusScraping | bool | `true` | Enable Prometheus metrics scraping on sidecar proxies |
 | OpenServiceMesh.enforceSingleMesh | bool | `false` | Enforce only deploying one mesh in the cluster |
 | OpenServiceMesh.envoyLogLevel | string | `"error"` | Log level for the Envoy proxy sidecar |
 | OpenServiceMesh.featureFlags.enableEgressPolicy | bool | `true` | Enable OSM's Egress policy API. If specified, fine grained control over Egress (external) traffic is enforced |

charts/osm/crds/meshconfig.yaml

@@ -159,10 +159,6 @@ spec:
                       description: Enables a debug endpoint on the osm-controller pod to list information regarding the mesh such as proxy connections, certificates, and SMI policies.
                       type: boolean
                       default: false
-                    prometheusScraping:
-                      description: Enables Prometheus metrics scraping on sidecar proxies.
-                      type: boolean
-                      default: true
                     tracing:
                       description: Configuration for distributed tracing
                       type: object

charts/osm/templates/preset-mesh-config.yaml

@@ -19,7 +19,6 @@ spec:
     outboundIPRangeExclusionList: {{.Values.OpenServiceMesh.outboundIPRangeExclusionList}}
   observability:
     enableDebugServer: {{.Values.OpenServiceMesh.enableDebugServer}}
-    prometheusScraping: {{.Values.OpenServiceMesh.enablePrometheusScraping}}
     tracing:
       enable: {{.Values.OpenServiceMesh.tracing.enable}}
       {{- if .Values.OpenServiceMesh.tracing.enable }}

charts/osm/values.schema.json

@@ -85,7 +85,6 @@
                 "enablePermissiveTrafficPolicy",
                 "enableEgress",
                 "deployPrometheus",
-                "enablePrometheusScraping",
                 "deployGrafana",
                 "enableFluentbit",
                 "fluentBit",
@@ -256,15 +255,6 @@
                         false
                     ]
                 },
-                "enablePrometheusScraping": {
-                    "$id": "#/properties/OpenServiceMesh/properties/enablePrometheusScraping",
-                    "type": "boolean",
-                    "title": "The enablePrometheusScraping schema",
-                    "description": "Indicates whether Prometheus scraping should be enabled.",
-                    "examples": [
-                        true
-                    ]
-                },
                 "deployGrafana": {
                     "$id": "#/properties/OpenServiceMesh/properties/deployGrafana",
                     "type": "boolean",

charts/osm/values.yaml

@@ -104,9 +104,6 @@ OpenServiceMesh:
   # -- Deploy Prometheus with OSM installation
   deployPrometheus: false
 
-  # -- Enable Prometheus metrics scraping on sidecar proxies
-  enablePrometheusScraping: true
-
   # -- Deploy Grafana with OSM installation
   deployGrafana: false
 

cmd/cli/install.go

@@ -233,13 +233,6 @@ func (i *installCmd) validateOptions() error {
 	}
 
 	if setOptions, ok := s["OpenServiceMesh"].(map[string]interface{}); ok {
-		// if deployPrometheus is true, make sure enablePrometheusScraping is not disabled
-		if setOptions["deployPrometheus"] == true {
-			if setOptions["enablePrometheusScraping"] == false {
-				_, _ = fmt.Fprintf(i.out, "Prometheus scraping is disabled. To enable it, set prometheus_scraping in %s/%s to true.\n", settings.Namespace(), constants.OSMMeshConfig)
-			}
-		}
-
 		// if certificateManager is vault, ensure all relevant information (vault-host, vault-token) is available
 		if setOptions["certificateManager"] == "vault" {
 			var missingFields []string

cmd/cli/install_test.go

@@ -534,15 +534,13 @@ var _ = Describe("deployPrometheus is true", func() {
 		installCmd := getDefaultInstallCmd(out)
 		installCmd.setOptions = []string{
 			"OpenServiceMesh.deployPrometheus=true",
-			"OpenServiceMesh.enablePrometheusScraping=false",
 		}
 
 		err = installCmd.run(config)
 	})
 
 	It("should not error", func() {
 		Expect(err).NotTo(HaveOccurred())
-		Expect(out.String()).To(Equal("Prometheus scraping is disabled. To enable it, set prometheus_scraping in osm-system/osm-mesh-config to true.\nOSM installed successfully in namespace [osm-system] with mesh name [osm]\n"))
 	})
 })
 

cmd/init-osm-controller/init-osm-controller_test.go

@@ -35,8 +35,7 @@ func TestCreateDefaultMeshConfig(t *testing.T) {
 				EnablePermissiveTrafficPolicyMode: true,
 			},
 			Observability: v1alpha1.ObservabilitySpec{
-				EnableDebugServer:  false,
-				PrometheusScraping: true,
+				EnableDebugServer: false,
 				Tracing: v1alpha1.TracingSpec{
 					Enable: false,
 				},
@@ -55,7 +54,6 @@ func TestCreateDefaultMeshConfig(t *testing.T) {
 	assert.Equal(meshConfig.Spec.Traffic.EnablePermissiveTrafficPolicyMode, true)
 	assert.Equal(meshConfig.Spec.Traffic.EnableEgress, true)
 	assert.Equal(meshConfig.Spec.Traffic.UseHTTPSIngress, false)
-	assert.Equal(meshConfig.Spec.Observability.PrometheusScraping, true)
 	assert.Equal(meshConfig.Spec.Observability.EnableDebugServer, false)
 	assert.Equal(meshConfig.Spec.Certificate.ServiceCertValidityDuration, "24h")
 }

demo/run-osm-demo.sh

@@ -33,7 +33,6 @@ DEPLOY_GRAFANA="${DEPLOY_GRAFANA:-false}"
 DEPLOY_JAEGER="${DEPLOY_JAEGER:-false}"
 ENABLE_FLUENTBIT="${ENABLE_FLUENTBIT:-false}"
 DEPLOY_PROMETHEUS="${DEPLOY_PROMETHEUS:-false}"
-ENABLE_PROMETHEUS_SCRAPING="${ENABLE_PROMETHEUS_SCRAPING:-true}"
 DEPLOY_WITH_SAME_SA="${DEPLOY_WITH_SAME_SA:-false}"
 ENVOY_LOG_LEVEL="${ENVOY_LOG_LEVEL:-debug}"
 DEPLOY_ON_OPENSHIFT="${DEPLOY_ON_OPENSHIFT:-false}"
@@ -107,7 +106,6 @@ if [ "$CERT_MANAGER" = "vault" ]; then
       --set=OpenServiceMesh.deployJaeger="$DEPLOY_JAEGER" \
       --set=OpenServiceMesh.enableFluentbit="$ENABLE_FLUENTBIT" \
       --set=OpenServiceMesh.deployPrometheus="$DEPLOY_PROMETHEUS" \
-      --set=OpenServiceMesh.enablePrometheusScraping="$ENABLE_PROMETHEUS_SCRAPING" \
       --set=OpenServiceMesh.envoyLogLevel="$ENVOY_LOG_LEVEL" \
       --set=OpenServiceMesh.controllerLogLevel="trace" \
       --timeout=90s \
@@ -128,7 +126,6 @@ else
       --set=OpenServiceMesh.deployJaeger="$DEPLOY_JAEGER" \
       --set=OpenServiceMesh.enableFluentbit="$ENABLE_FLUENTBIT" \
       --set=OpenServiceMesh.deployPrometheus="$DEPLOY_PROMETHEUS" \
-      --set=OpenServiceMesh.enablePrometheusScraping="$ENABLE_PROMETHEUS_SCRAPING" \
       --set=OpenServiceMesh.envoyLogLevel="$ENVOY_LOG_LEVEL" \
       --set=OpenServiceMesh.controllerLogLevel="trace" \
       --timeout=90s \

docs/example/manifests/meshconfig/mesh-config.yaml

@@ -16,7 +16,6 @@ spec:
     enablePermissiveTrafficPolicyMode: true
   observability:
     enableDebugServer: true
-    prometheusScraping: true
     outboundPortExclusionList: []
     inboundPortExclusionList: []
     outboundIPRangeExclusionList: []

pkg/apis/config/v1alpha1/mesh_config.go

@@ -94,9 +94,6 @@ type ObservabilitySpec struct {
 	// EnableDebugServer defines if the debug endpoint on the OSM controller pod is enabled.
 	EnableDebugServer bool `json:"enableDebugServer,omitempty"`
 
-	// PrometheusScraping defines a boolean indicating if sidecars should be configured for Prometheus metrics scraping.
-	PrometheusScraping bool `json:"prometheusScraping,omitempty"`
-
 	// Tracing defines OSM's tracing configuration.
 	Tracing TracingSpec `json:"tracing,omitempty"`
 }

pkg/catalog/catalog.go

@@ -36,3 +36,8 @@ func NewMeshCatalog(kubeController k8s.Controller, kubeClient kubernetes.Interfa
 
 	return &mc
 }
+
+// GetKubeController returns the kube controller instance handling the current cluster
+func (mc *MeshCatalog) GetKubeController() k8s.Controller {
+	return mc.kubeController
+}

pkg/catalog/fake.go

@@ -109,6 +109,7 @@ func NewFakeMeshCatalog(kubeClient kubernetes.Interface, meshConfigClient versio
 	mockKubeController.EXPECT().ListServiceIdentitiesForService(tests.BookstoreV1Service).Return([]identity.K8sServiceAccount{tests.BookstoreServiceAccount}, nil).AnyTimes()
 	mockKubeController.EXPECT().ListServiceIdentitiesForService(tests.BookstoreV2Service).Return([]identity.K8sServiceAccount{tests.BookstoreV2ServiceAccount}, nil).AnyTimes()
 	mockKubeController.EXPECT().ListServiceIdentitiesForService(tests.BookbuyerService).Return([]identity.K8sServiceAccount{tests.BookbuyerServiceAccount}, nil).AnyTimes()
+	mockKubeController.EXPECT().IsMetricsEnabled(gomock.Any()).Return(true).AnyTimes()
 
 	mockPolicyController.EXPECT().ListEgressPoliciesForSourceIdentity(gomock.Any()).Return(nil).AnyTimes()
 

pkg/catalog/types.go

@@ -99,6 +99,9 @@ type MeshCataloger interface {
 
 	// GetEgressTrafficPolicy returns the Egress traffic policy associated with the given service identity.
 	GetEgressTrafficPolicy(identity.ServiceIdentity) (*trafficpolicy.EgressTrafficPolicy, error)
+
+	// GetKubeController returns the kube controller instance handling the current cluster
+	GetKubeController() k8s.Controller
 }
 
 type trafficDirection string

pkg/configurator/methods.go

@@ -60,11 +60,6 @@ func (c *Client) IsDebugServerEnabled() bool {
 	return c.getMeshConfig().Spec.Observability.EnableDebugServer
 }
 
-// IsPrometheusScrapingEnabled determines whether Prometheus is enabled for scraping metrics
-func (c *Client) IsPrometheusScrapingEnabled() bool {
-	return c.getMeshConfig().Spec.Observability.PrometheusScraping
-}
-
 // IsTracingEnabled returns whether tracing is enabled
 func (c *Client) IsTracingEnabled() bool {
 	return c.getMeshConfig().Spec.Observability.Tracing.Enable

pkg/configurator/methods_test.go

@@ -62,8 +62,7 @@ func TestCreateUpdateConfig(t *testing.T) {
 					UseHTTPSIngress:                   true,
 				},
 				Observability: v1alpha1.ObservabilitySpec{
-					EnableDebugServer:  true,
-					PrometheusScraping: true,
+					EnableDebugServer: true,
 					Tracing: v1alpha1.TracingSpec{
 						Enable: true,
 					},
@@ -88,8 +87,7 @@ func TestCreateUpdateConfig(t *testing.T) {
 						UseHTTPSIngress:                   true,
 					},
 					Observability: v1alpha1.ObservabilitySpec{
-						EnableDebugServer:  true,
-						PrometheusScraping: true,
+						EnableDebugServer: true,
 						Tracing: v1alpha1.TracingSpec{
 							Enable: true,
 						},
@@ -163,25 +161,6 @@ func TestCreateUpdateConfig(t *testing.T) {
 				assert.False(cfg.IsDebugServerEnabled())
 			},
 		},
-		{
-			name: "IsPrometheusScrapingEnabled",
-			initialMeshConfigData: &v1alpha1.MeshConfigSpec{
-				Observability: v1alpha1.ObservabilitySpec{
-					PrometheusScraping: true,
-				},
-			},
-			checkCreate: func(assert *tassert.Assertions, cfg Configurator) {
-				assert.True(cfg.IsPrometheusScrapingEnabled())
-			},
-			updatedMeshConfigData: &v1alpha1.MeshConfigSpec{
-				Observability: v1alpha1.ObservabilitySpec{
-					PrometheusScraping: false,
-				},
-			},
-			checkUpdate: func(assert *tassert.Assertions, cfg Configurator) {
-				assert.False(cfg.IsPrometheusScrapingEnabled())
-			},
-		},
 		{
 			name: "IsTracingEnabled",
 			initialMeshConfigData: &v1alpha1.MeshConfigSpec{

pkg/configurator/types.go

@@ -43,9 +43,6 @@ type Configurator interface {
 	// IsDebugServerEnabled determines whether osm debug HTTP server is enabled
 	IsDebugServerEnabled() bool
 
-	// IsPrometheusScrapingEnabled determines whether Prometheus is enabled for scraping metrics
-	IsPrometheusScrapingEnabled() bool
-
 	// IsTracingEnabled returns whether tracing is enabled
 	IsTracingEnabled() bool
 

pkg/envoy/ads/response_test.go

@@ -125,7 +125,6 @@ var _ = Describe("Test ADS response functions", func() {
 		server, actualResponses := tests.NewFakeXDSServer(cert, nil, nil)
 
 		mockConfigurator.EXPECT().IsEgressEnabled().Return(false).AnyTimes()
-		mockConfigurator.EXPECT().IsPrometheusScrapingEnabled().Return(false).AnyTimes()
 		mockConfigurator.EXPECT().IsTracingEnabled().Return(false).AnyTimes()
 		mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(false).AnyTimes()
 		mockConfigurator.EXPECT().GetServiceCertValidityPeriod().Return(certDuration).AnyTimes()
@@ -209,7 +208,6 @@ var _ = Describe("Test ADS response functions", func() {
 		server, actualResponses := tests.NewFakeXDSServer(cert, nil, nil)
 
 		mockConfigurator.EXPECT().IsEgressEnabled().Return(false).AnyTimes()
-		mockConfigurator.EXPECT().IsPrometheusScrapingEnabled().Return(false).AnyTimes()
 		mockConfigurator.EXPECT().IsTracingEnabled().Return(false).AnyTimes()
 		mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(false).AnyTimes()
 		mockConfigurator.EXPECT().GetServiceCertValidityPeriod().Return(certDuration).AnyTimes()

pkg/envoy/cds/response.go

@@ -75,7 +75,9 @@ func NewResponse(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_d
 	}
 
 	// Add an inbound prometheus cluster (from Prometheus to localhost)
-	if cfg.IsPrometheusScrapingEnabled() {
+	if pod, err := envoy.GetPodFromCertificate(proxy.GetCertificateCommonName(), meshCatalog.GetKubeController()); err != nil {
+		log.Warn().Msgf("Could not find pod for connecting proxy %s. No metadata was recorded.", proxy.GetCertificateSerialNumber())
+	} else if meshCatalog.GetKubeController().IsMetricsEnabled(pod) {
 		clusters = append(clusters, getPrometheusCluster())
 	}
 

pkg/envoy/cds/response_test.go

@@ -1,6 +1,7 @@
 package cds
 
 import (
+	"context"
 	"fmt"
 	"testing"
 	"time"
@@ -19,6 +20,9 @@ import (
 	tassert "github.com/stretchr/testify/assert"
 	trequire "github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/testing/protocmp"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	testclient "k8s.io/client-go/kubernetes/fake"
 
 	"github.com/openservicemesh/osm/pkg/catalog"
 	"github.com/openservicemesh/osm/pkg/certificate"
@@ -27,6 +31,7 @@ import (
 	"github.com/openservicemesh/osm/pkg/envoy"
 	"github.com/openservicemesh/osm/pkg/envoy/registry"
 	"github.com/openservicemesh/osm/pkg/envoy/secrets"
+	k8s "github.com/openservicemesh/osm/pkg/kubernetes"
 	"github.com/openservicemesh/osm/pkg/service"
 	"github.com/openservicemesh/osm/pkg/tests"
 )
@@ -36,8 +41,10 @@ func TestNewResponse(t *testing.T) {
 	require := trequire.New(t)
 
 	mockCtrl := gomock.NewController(t)
+	kubeClient := testclient.NewSimpleClientset()
 	mockConfigurator := configurator.NewMockConfigurator(mockCtrl)
 	mockCatalog := catalog.NewMockMeshCataloger(mockCtrl)
+	mockKubeController := k8s.NewMockController(mockCtrl)
 
 	proxyUUID := uuid.New()
 	// The format of the CN matters
@@ -53,10 +60,25 @@ func TestNewResponse(t *testing.T) {
 	mockCatalog.EXPECT().GetEgressTrafficPolicy(tests.BookbuyerServiceIdentity).Return(nil, nil).AnyTimes()
 	mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(false).AnyTimes()
 	mockConfigurator.EXPECT().IsEgressEnabled().Return(true).AnyTimes()
-	mockConfigurator.EXPECT().IsPrometheusScrapingEnabled().Return(true).AnyTimes()
 	mockConfigurator.EXPECT().IsTracingEnabled().Return(true).AnyTimes()
 	mockConfigurator.EXPECT().GetTracingHost().Return(constants.DefaultTracingHost).AnyTimes()
 	mockConfigurator.EXPECT().GetTracingPort().Return(constants.DefaultTracingPort).AnyTimes()
+	mockCatalog.EXPECT().GetKubeController().Return(mockKubeController).AnyTimes()
+
+	podlabels := map[string]string{
+		tests.SelectorKey:                tests.BookbuyerServiceName,
+		constants.EnvoyUniqueIDLabelName: proxyUUID.String(),
+	}
+
+	newPod1 := tests.NewPodFixture(tests.Namespace, fmt.Sprintf("pod-1-%s", proxyUUID), tests.BookbuyerServiceAccountName, podlabels)
+	newPod1.Annotations = map[string]string{
+		constants.PrometheusScrapeAnnotation: "true",
+	}
+	_, err := kubeClient.CoreV1().Pods(tests.Namespace).Create(context.TODO(), &newPod1, metav1.CreateOptions{})
+	assert.Nil(err)
+
+	mockKubeController.EXPECT().ListPods().Return([]*v1.Pod{&newPod1})
+	mockKubeController.EXPECT().IsMetricsEnabled(&newPod1).Return(true)
 
 	resp, err := NewResponse(mockCatalog, proxy, nil, mockConfigurator, nil, proxyRegistry)
 	assert.Nil(err)