diff --git a/.env.example b/.env.example index bdd7e5d318..abd5094a9f 100644 --- 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 index e62b3c09f9..965d8ebe36 100644 --- 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 index 961d64e7e7..69199297d7 100644 --- 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 index 2f1761dba5..69e0fcc410 100644 --- 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 index 3944765d29..4e8869adec 100644 --- 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 index a9a3b5512e..7b9a512484 100644 --- 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 index ea1257cd86..2245e09f46 100644 --- 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 index 6f61a34dd6..79aa231f1c 100644 --- a/cmd/cli/install_test.go +++ b/cmd/cli/install_test.go @@ -534,7 +534,6 @@ var _ = Describe("deployPrometheus is true", func() { installCmd := getDefaultInstallCmd(out) installCmd.setOptions = []string{ "OpenServiceMesh.deployPrometheus=true", - "OpenServiceMesh.enablePrometheusScraping=false", } err = installCmd.run(config) @@ -542,7 +541,6 @@ var _ = Describe("deployPrometheus is true", func() { 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 index 221a1b553e..8fc1985e13 100644 --- 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 index e810d04fb0..2e43c78581 100755 --- 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 index f10c4ba20f..c815ef290f 100644 --- 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 index 3ff917795b..024a9018c8 100644 --- 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 index 55336386ef..fc71c43130 100644 --- 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 index 98f5c70999..17e461ed33 100644 --- 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 index 3f5b49f246..e61bf7713f 100644 --- a/pkg/catalog/mock_catalog_generated.go +++ b/pkg/catalog/mock_catalog_generated.go @@ -10,6 +10,7 @@ import ( gomock "github.com/golang/mock/gomock" endpoint "github.com/openservicemesh/osm/pkg/endpoint" identity "github.com/openservicemesh/osm/pkg/identity" + kubernetes "github.com/openservicemesh/osm/pkg/kubernetes" service "github.com/openservicemesh/osm/pkg/service" trafficpolicy "github.com/openservicemesh/osm/pkg/trafficpolicy" ) @@ -67,6 +68,20 @@ func (mr *MockMeshCatalogerMockRecorder) GetIngressPoliciesForService(arg0 inter return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetIngressPoliciesForService", reflect.TypeOf((*MockMeshCataloger)(nil).GetIngressPoliciesForService), arg0) } +// GetKubeController mocks base method +func (m *MockMeshCataloger) GetKubeController() kubernetes.Controller { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetKubeController") + ret0, _ := ret[0].(kubernetes.Controller) + return ret0 +} + +// GetKubeController indicates an expected call of GetKubeController +func (mr *MockMeshCatalogerMockRecorder) GetKubeController() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetKubeController", reflect.TypeOf((*MockMeshCataloger)(nil).GetKubeController)) +} + // GetPortToProtocolMappingForService mocks base method func (m *MockMeshCataloger) GetPortToProtocolMappingForService(arg0 service.MeshService) (map[uint32]string, error) { m.ctrl.T.Helper() diff --git a/pkg/catalog/types.go b/pkg/catalog/types.go index a70d74935f..4d62746405 100644 --- 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 index 8213d8ea28..b1d09e76ee 100644 --- 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 index 2776a3e3f8..e384e7aaf9 100644 --- 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 index 9d6374fe63..89fa0d9677 100644 --- a/pkg/configurator/mock_client_generated.go +++ b/pkg/configurator/mock_client_generated.go @@ -303,20 +303,6 @@ func (mr *MockConfiguratorMockRecorder) IsPrivilegedInitContainer() *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsPrivilegedInitContainer", reflect.TypeOf((*MockConfigurator)(nil).IsPrivilegedInitContainer)) } -// IsPrometheusScrapingEnabled mocks base method -func (m *MockConfigurator) IsPrometheusScrapingEnabled() bool { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsPrometheusScrapingEnabled") - ret0, _ := ret[0].(bool) - return ret0 -} - -// IsPrometheusScrapingEnabled indicates an expected call of IsPrometheusScrapingEnabled -func (mr *MockConfiguratorMockRecorder) IsPrometheusScrapingEnabled() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsPrometheusScrapingEnabled", reflect.TypeOf((*MockConfigurator)(nil).IsPrometheusScrapingEnabled)) -} - // IsTracingEnabled mocks base method func (m *MockConfigurator) IsTracingEnabled() bool { m.ctrl.T.Helper() diff --git a/pkg/configurator/types.go b/pkg/configurator/types.go index 64f8975e0e..9940f1b359 100644 --- 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 index b0742e0319..819d3d622c 100644 --- 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 index e3f1d20e23..afaf210f84 100644 --- 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()) } diff --git a/pkg/envoy/cds/response_test.go b/pkg/envoy/cds/response_test.go index 87e7a9eed7..3789abf134 100644 --- a/pkg/envoy/cds/response_test.go +++ b/pkg/envoy/cds/response_test.go @@ -1,6 +1,7 @@ package cds import ( + "context" "fmt" "testing" "time" @@ -20,6 +21,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" @@ -29,6 +33,7 @@ import ( "github.com/openservicemesh/osm/pkg/envoy/registry" "github.com/openservicemesh/osm/pkg/envoy/secrets" "github.com/openservicemesh/osm/pkg/identity" + k8s "github.com/openservicemesh/osm/pkg/kubernetes" "github.com/openservicemesh/osm/pkg/service" "github.com/openservicemesh/osm/pkg/tests" "github.com/openservicemesh/osm/pkg/trafficpolicy" @@ -39,8 +44,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 @@ -56,10 +63,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) @@ -404,11 +426,13 @@ func TestNewResponseGetEgressTrafficPolicyError(t *testing.T) { ctrl := gomock.NewController(t) meshCatalog := catalog.NewMockMeshCataloger(ctrl) + mockKubeController := k8s.NewMockController(ctrl) meshCatalog.EXPECT().ListAllowedOutboundServicesForIdentity(proxyIdentity).Return(nil).Times(1) meshCatalog.EXPECT().GetEgressTrafficPolicy(proxyIdentity).Return(nil, errors.New("some error")).Times(1) + meshCatalog.EXPECT().GetKubeController().Return(mockKubeController).AnyTimes() + mockKubeController.EXPECT().ListPods().Return([]*v1.Pod{}) cfg := configurator.NewMockConfigurator(ctrl) cfg.EXPECT().IsEgressEnabled().Return(false).Times(1) - cfg.EXPECT().IsPrometheusScrapingEnabled().Return(false).Times(1) cfg.EXPECT().IsTracingEnabled().Return(false).Times(1) resp, err := NewResponse(meshCatalog, proxy, nil, cfg, nil, proxyRegistry) @@ -426,7 +450,10 @@ func TestNewResponseGetEgressTrafficPolicyNotEmpty(t *testing.T) { ctrl := gomock.NewController(t) meshCatalog := catalog.NewMockMeshCataloger(ctrl) + mockKubeController := k8s.NewMockController(ctrl) meshCatalog.EXPECT().ListAllowedOutboundServicesForIdentity(proxyIdentity).Return(nil).Times(1) + meshCatalog.EXPECT().GetKubeController().Return(mockKubeController).AnyTimes() + mockKubeController.EXPECT().ListPods().Return([]*v1.Pod{}) meshCatalog.EXPECT().GetEgressTrafficPolicy(proxyIdentity).Return(&trafficpolicy.EgressTrafficPolicy{ ClustersConfigs: []*trafficpolicy.EgressClusterConfig{ {Name: "my-cluster"}, @@ -435,7 +462,6 @@ func TestNewResponseGetEgressTrafficPolicyNotEmpty(t *testing.T) { }, nil).Times(1) cfg := configurator.NewMockConfigurator(ctrl) cfg.EXPECT().IsEgressEnabled().Return(false).Times(1) - cfg.EXPECT().IsPrometheusScrapingEnabled().Return(false).Times(1) cfg.EXPECT().IsTracingEnabled().Return(false).Times(1) resp, err := NewResponse(meshCatalog, proxy, nil, cfg, nil, proxyRegistry) diff --git a/pkg/envoy/lds/response.go b/pkg/envoy/lds/response.go index 2333d5cad0..fc1cfc2fcc 100644 --- a/pkg/envoy/lds/response.go +++ b/pkg/envoy/lds/response.go @@ -86,7 +86,9 @@ func NewResponse(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_d ldsResources = append(ldsResources, inboundListener) } - 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) { // Build Prometheus listener config prometheusConnManager := getPrometheusConnectionManager() if prometheusListener, err := buildPrometheusListener(prometheusConnManager); err != nil { diff --git a/pkg/envoy/lds/response_test.go b/pkg/envoy/lds/response_test.go index e02e1113fd..2260d16c24 100644 --- a/pkg/envoy/lds/response_test.go +++ b/pkg/envoy/lds/response_test.go @@ -1,6 +1,7 @@ package lds import ( + "context" "fmt" "testing" @@ -9,6 +10,7 @@ import ( "github.com/envoyproxy/go-control-plane/pkg/wellknown" "github.com/golang/mock/gomock" tassert "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" testclient "k8s.io/client-go/kubernetes/fake" @@ -30,7 +32,12 @@ func getProxy(kubeClient kubernetes.Interface) (*envoy.Proxy, error) { tests.SelectorKey: tests.BookbuyerService.Name, constants.EnvoyUniqueIDLabelName: tests.ProxyUUID, } - if _, err := tests.MakePod(kubeClient, tests.Namespace, tests.BookbuyerServiceName, tests.BookbuyerServiceAccountName, podLabels); err != nil { + + newPod1 := tests.NewPodFixture(tests.Namespace, tests.BookbuyerServiceName, tests.BookbuyerServiceAccountName, podLabels) + newPod1.Annotations = map[string]string{ + constants.PrometheusScrapeAnnotation: "true", + } + if _, err := kubeClient.CoreV1().Pods(tests.Namespace).Create(context.TODO(), &newPod1, metav1.CreateOptions{}); err != nil { return nil, err } @@ -66,7 +73,6 @@ func TestNewResponse(t *testing.T) { proxy, err := getProxy(kubeClient) assert.Empty(err) - assert.NotNil(meshCatalog) assert.NotNil(proxy) // test scenario that listing proxy services returns an error @@ -82,7 +88,6 @@ func TestNewResponse(t *testing.T) { })) mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(false).AnyTimes() - mockConfigurator.EXPECT().IsPrometheusScrapingEnabled().Return(true).AnyTimes() mockConfigurator.EXPECT().IsTracingEnabled().Return(false).AnyTimes() mockConfigurator.EXPECT().IsEgressEnabled().Return(true).AnyTimes() mockConfigurator.EXPECT().GetInboundExternalAuthConfig().Return(auth.ExtAuthConfig{ diff --git a/pkg/envoy/xdsutil.go b/pkg/envoy/xdsutil.go index 49827f60d8..c9637a342a 100644 --- a/pkg/envoy/xdsutil.go +++ b/pkg/envoy/xdsutil.go @@ -1,7 +1,6 @@ package envoy import ( - "errors" "fmt" "strings" @@ -16,6 +15,7 @@ import ( structpb "github.com/golang/protobuf/ptypes/struct" "github.com/golang/protobuf/ptypes/wrappers" "github.com/google/uuid" + "github.com/pkg/errors" v1 "k8s.io/api/core/v1" "github.com/openservicemesh/osm/pkg/certificate" diff --git a/pkg/kubernetes/client.go b/pkg/kubernetes/client.go index 717115f745..53a3633103 100644 --- a/pkg/kubernetes/client.go +++ b/pkg/kubernetes/client.go @@ -2,6 +2,7 @@ package kubernetes import ( "reflect" + "strconv" mapset "github.com/deckarep/golang-set" "github.com/pkg/errors" @@ -298,3 +299,15 @@ func (c Client) ListServiceIdentitiesForService(svc service.MeshService) ([]iden } return svcAccounts, nil } + +// IsMetricsEnabled returns true if the pod in the mesh is correctly annotated for prometheus scrapping +func (c Client) IsMetricsEnabled(pod *corev1.Pod) bool { + isScrapingEnabled := false + prometheusScrapeAnnotation, ok := pod.Annotations[constants.PrometheusScrapeAnnotation] + if !ok { + return isScrapingEnabled + } + + isScrapingEnabled, _ = strconv.ParseBool(prometheusScrapeAnnotation) + return isScrapingEnabled +} diff --git a/pkg/kubernetes/client_test.go b/pkg/kubernetes/client_test.go index 478d60fc97..373ee5c11b 100644 --- a/pkg/kubernetes/client_test.go +++ b/pkg/kubernetes/client_test.go @@ -613,3 +613,79 @@ func TestGetEndpoint(t *testing.T) { assert.Nil(err) assert.Nil(endpoint) } + +func TestIsMetricsEnabled(t *testing.T) { + assert := tassert.New(t) + + testCases := []struct { + name string + addPrometheusAnnotation bool + expectedMetricsScraping bool + scrapingAnnotation string + }{ + { + name: "pod without prometheus scraping annotation", + scrapingAnnotation: "false", + addPrometheusAnnotation: false, + expectedMetricsScraping: false, + }, + { + name: "pod with prometheus scraping annotation set to true", + scrapingAnnotation: "true", + addPrometheusAnnotation: true, + expectedMetricsScraping: true, + }, + { + name: "pod with prometheus scraping annotation set to false", + scrapingAnnotation: "false", + addPrometheusAnnotation: true, + expectedMetricsScraping: false, + }, + { + name: "pod with incorrect prometheus scraping annotation", + scrapingAnnotation: "no", + addPrometheusAnnotation: true, + expectedMetricsScraping: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + kubeClient := testclient.NewSimpleClientset() + stop := make(chan struct{}) + kubeController, err := NewKubernetesController(kubeClient, testMeshName, stop) + assert.Nil(err) + assert.NotNil(kubeController) + + proxyUUID := uuid.New() + namespace := uuid.New().String() + podlabels := map[string]string{ + tests.SelectorKey: tests.SelectorValue, + constants.EnvoyUniqueIDLabelName: proxyUUID.String(), + } + + // Ensure correct presetup + pods, err := kubeClient.CoreV1().Pods(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.Nil(err) + assert.Len(pods.Items, 0) + + newPod1 := tests.NewPodFixture(namespace, fmt.Sprintf("pod-1-%s", proxyUUID), tests.BookstoreServiceAccountName, podlabels) + + if tc.addPrometheusAnnotation { + newPod1.Annotations = map[string]string{ + constants.PrometheusScrapeAnnotation: tc.scrapingAnnotation, + } + } + _, err = kubeClient.CoreV1().Pods(namespace).Create(context.TODO(), &newPod1, metav1.CreateOptions{}) + assert.Nil(err) + + // Ensure correct setup + pods, err = kubeClient.CoreV1().Pods(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.Nil(err) + assert.Len(pods.Items, 1) + + actual := kubeController.IsMetricsEnabled(&newPod1) + assert.Equal(actual, tc.expectedMetricsScraping) + }) + } +} diff --git a/pkg/kubernetes/mock_controller_generated.go b/pkg/kubernetes/mock_controller_generated.go index 0b0c2b5276..eae6311970 100644 --- a/pkg/kubernetes/mock_controller_generated.go +++ b/pkg/kubernetes/mock_controller_generated.go @@ -79,6 +79,20 @@ func (mr *MockControllerMockRecorder) GetService(arg0 interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetService", reflect.TypeOf((*MockController)(nil).GetService), arg0) } +// IsMetricsEnabled mocks base method +func (m *MockController) IsMetricsEnabled(arg0 *v1.Pod) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsMetricsEnabled", arg0) + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsMetricsEnabled indicates an expected call of IsMetricsEnabled +func (mr *MockControllerMockRecorder) IsMetricsEnabled(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsMetricsEnabled", reflect.TypeOf((*MockController)(nil).IsMetricsEnabled), arg0) +} + // IsMonitoredNamespace mocks base method func (m *MockController) IsMonitoredNamespace(arg0 string) bool { m.ctrl.T.Helper() diff --git a/pkg/kubernetes/types.go b/pkg/kubernetes/types.go index d5227b5e4e..a839b6bfbb 100644 --- a/pkg/kubernetes/types.go +++ b/pkg/kubernetes/types.go @@ -100,4 +100,7 @@ type Controller interface { // GetEndpoints returns the endpoints for a given service, if found GetEndpoints(svc service.MeshService) (*corev1.Endpoints, error) + + // IsMetricsEnabled returns true if the pod in the mesh is correctly annotated for prometheus scrapping + IsMetricsEnabled(*corev1.Pod) bool } diff --git a/tests/e2e/e2e_helm_install_test.go b/tests/e2e/e2e_helm_install_test.go index dfc63791f2..fe597606b0 100644 --- a/tests/e2e/e2e_helm_install_test.go +++ b/tests/e2e/e2e_helm_install_test.go @@ -34,7 +34,6 @@ var _ = OSMDescribe("Test osm control plane installation with Helm", Expect(spec.Traffic.EnableEgress).To(BeFalse()) Expect(spec.Sidecar.LogLevel).To(Equal("error")) Expect(spec.Observability.EnableDebugServer).To(BeFalse()) - Expect(spec.Observability.PrometheusScraping).To(BeTrue()) Expect(spec.Observability.Tracing.Enable).To(BeFalse()) Expect(spec.Traffic.UseHTTPSIngress).To(BeFalse()) Expect(spec.Certificate.ServiceCertValidityDuration).To(Equal("24h")) diff --git a/tests/e2e/e2e_init_controller_test.go b/tests/e2e/e2e_init_controller_test.go index 0b9937a6bd..5e3518f777 100644 --- a/tests/e2e/e2e_init_controller_test.go +++ b/tests/e2e/e2e_init_controller_test.go @@ -26,7 +26,6 @@ var _ = OSMDescribe("Test init-osm-controller functionalities", Expect(meshConfig.Spec.Traffic.EnablePermissiveTrafficPolicyMode).Should(BeFalse()) Expect(meshConfig.Spec.Traffic.EnableEgress).Should(BeFalse()) Expect(meshConfig.Spec.Sidecar.LogLevel).Should(Equal("debug")) - Expect(meshConfig.Spec.Observability.PrometheusScraping).Should(BeTrue()) Expect(meshConfig.Spec.Observability.EnableDebugServer).Should(BeTrue()) Expect(meshConfig.Spec.Observability.Tracing.Enable).Should(BeFalse()) Expect(meshConfig.Spec.Traffic.UseHTTPSIngress).Should(BeFalse())