prometheusScraping: Remove prometheus scraping flag

This commit removes the prometheus_scraping flag and adds the envoy prometheus scraping configs when a pod has the required prometheus annotation. Part of #3065 Signed-off-by: Sneha Chhabria <[email protected]>
openservicemesh · Jun 4, 2021 · 3f8d154 · 3f8d154
1 parent 722614f
commit 3f8d154
Show file tree

Hide file tree

Showing 32 changed files with 177 additions and 100 deletions.
diff --git a/.env.example b/.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
 

diff --git a/charts/osm/README.md b/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 |

diff --git a/charts/osm/crds/meshconfig.yaml b/charts/osm/crds/meshconfig.yaml
@@ -153,10 +153,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

diff --git a/charts/osm/templates/preset-mesh-config.yaml b/charts/osm/templates/preset-mesh-config.yaml
@@ -18,7 +18,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 }}

diff --git a/charts/osm/values.schema.json b/charts/osm/values.schema.json
@@ -85,7 +85,6 @@
                 "enablePermissiveTrafficPolicy",
                 "enableEgress",
                 "deployPrometheus",
-                "enablePrometheusScraping",
                 "deployGrafana",
                 "enableFluentbit",
                 "fluentBit",
@@ -265,15 +264,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",

diff --git a/charts/osm/values.yaml b/charts/osm/values.yaml
@@ -106,9 +106,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
 

diff --git a/cmd/cli/install.go b/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

diff --git a/cmd/cli/install_test.go b/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"))
 	})
 })
 

diff --git a/cmd/init-osm-controller/init-osm-controller_test.go b/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")
 }
diff --git a/demo/run-osm-demo.sh b/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 \

diff --git a/docs/example/manifests/meshconfig/mesh-config.yaml b/docs/example/manifests/meshconfig/mesh-config.yaml
@@ -16,7 +16,6 @@ spec:
     enablePermissiveTrafficPolicyMode: true
   observability:
     enableDebugServer: true
-    prometheusScraping: true
     outboundPortExclusionList: []
     outboundIPRangeExclusionList: []
     tracing:

diff --git a/pkg/apis/config/v1alpha1/mesh_config.go b/pkg/apis/config/v1alpha1/mesh_config.go
@@ -88,9 +88,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"`
 }

diff --git a/pkg/catalog/catalog.go b/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
+}
diff --git a/pkg/catalog/fake.go b/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()
 

diff --git a/pkg/catalog/mock_catalog_generated.go b/pkg/catalog/mock_catalog_generated.go
diff --git a/pkg/catalog/types.go b/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

diff --git a/pkg/configurator/methods.go b/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

diff --git a/pkg/configurator/methods_test.go b/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{

diff --git a/pkg/configurator/mock_client_generated.go b/pkg/configurator/mock_client_generated.go
diff --git a/pkg/configurator/types.go b/pkg/configurator/types.go
@@ -42,9 +42,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
 

diff --git a/pkg/envoy/ads/response_test.go b/pkg/envoy/ads/response_test.go
@@ -126,7 +126,6 @@ var _ = Describe("Test ADS response functions", func() {
 		kubectrlMock := kubernetes.NewMockController(mockCtrl)
 
 		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()
@@ -207,7 +206,6 @@ var _ = Describe("Test ADS response functions", func() {
 		kubectrlMock := kubernetes.NewMockController(mockCtrl)
 
 		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()

diff --git a/pkg/envoy/cds/response.go b/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())
 	}