From 74fe4ed608da89e26d09d0cda50364b6d81a375f Mon Sep 17 00:00:00 2001 From: AliceProxy Date: Wed, 16 Aug 2023 09:54:20 -0700 Subject: [PATCH] remove agent stats sink Signed-off-by: AliceProxy --- CHANGELOG.md | 40 ++++++++------ .../templates/deployment.yaml | 6 --- docs/releaseNotes.yml | 43 ++++++++------- manifests/emissary/emissary-defaultns.yaml.in | 6 --- .../emissary/emissary-emissaryns.yaml.in | 6 --- python/ambassador/envoy/v3/v3bootstrap.py | 54 ------------------- .../integration/manifests/ambassador.yaml | 6 --- 7 files changed, 48 insertions(+), 113 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ecefeed52a..3c24d58506 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,6 +105,12 @@ it will be removed; but as it won't be user-visible this isn't considered a brea be sure to re-apply any v3alpha1 Mappings in order to update the stored v2 Mapping and resolve the issue. +- Change: When the Ambassador agent is being used, it will no longer attempt to collect and report + Envoy metrics. In previous versions, Emissary-ingress would always create an Envoy stats sink for + the agent as long as the AMBASSADOR_GRPC_METRICS_SINK environmet variable was provided. This + environment variable was hardcoded on the release manifests and has now been removed and an Envoy + stats sink for the agent is no longer created. + [Canary grouping must take labels into account]: https://github.com/emissary-ingress/emissary/issues/4170 ## [3.7.2] July 25, 2023 @@ -113,7 +119,7 @@ it will be removed; but as it won't be user-visible this isn't considered a brea ### Emissary-ingress and Ambassador Edge Stack - Security: This upgrades Emissary-ingress to be built on Envoy v1.26.4 which includes a security - fixes for CVE-2023-35942, CVE-2023-35943, VE-2023-35944. + fixes for CVE-2023-35942, CVE-2023-35943, VE-2023-35944. ## [3.7.1] July 13, 2023 [3.7.1]: https://github.com/emissary-ingress/emissary/compare/v3.7.0...v3.7.1 @@ -152,10 +158,10 @@ it will be removed; but as it won't be user-visible this isn't considered a brea - Security: Upgrading to the latest release of Golang as part of our general dependency upgrade process. This includes security fixes for CVE-2022-41725, CVE-2022-41723. -- Feature: In Envoy 1.24, experimental support for a native OpenTelemetry tracing driver was - introduced that allows exporting spans in the otlp format. Many Observability platforms accept +- Feature: In Envoy 1.24, experimental support for a native OpenTelemetry tracing driver was + introduced that allows exporting spans in the otlp format. Many Observability platforms accept that format and is the recommend replacement for the LightStep driver. Emissary-ingress now - supports setting the `TracingService.spec.driver=opentelemetry` to export spans in otlp + supports setting the `TracingService.spec.driver=opentelemetry` to export spans in otlp format.

Thanks to Paul for helping us get this tested and implemented! @@ -174,14 +180,14 @@ it will be removed; but as it won't be user-visible this isn't considered a brea - Change: Previously, specifying backend ports by name in Ingress was not supported and would result in defaulting to port 80. This allows emissary-ingress to now resolve port names for backend services. If the port number cannot be resolved by the name (e.g named port in the Service doesn't - exist) then it defaults back to the original behavior. (Thanks to Anton Ustyuzhanin!). ([#4809]) -- Change: The `emissary-apiext` server is a Kubernetes Conversion Webhook that converts between the +- Change: The `emissary-apiext` server is a Kubernetes Conversion Webhook that converts between the Emissary-ingress CRD versions. On startup, it ensures that a self-signed cert is available so that - K8s API Server can talk to the conversion webhook (*TLS is required by K8s*). We have introduced - a startupProbe to ensure that emissary-apiext server has enough time to configure the webhooks - before running liveness and readiness probes. This is to ensure slow startup doesn't cause K8s to + K8s API Server can talk to the conversion webhook (*TLS is required by K8s*). We have introduced a + startupProbe to ensure that emissary-apiext server has enough time to configure the webhooks + before running liveness and readiness probes. This is to ensure slow startup doesn't cause K8s to needlessly restart the pod. [fix: hostname port issue]: https://github.com/emissary-ingress/emissary/pull/4816 @@ -213,17 +219,17 @@ it will be removed; but as it won't be user-visible this isn't considered a brea server to. - `AMBASSADOR_HEALTHCHECK_IP_FAMILY`: The IP family to use for the healthcheck server. - This allows the healthcheck server to be configured to use IPv6-only k8s environments. + This allows the healthcheck server to be configured to use IPv6-only k8s environments. (Thanks to Dmitry Golushko!). -- Feature: This upgrades Emissary-ingress to be built on Envoy v1.24.1. One notable change is that +- Feature: This upgrades Emissary-ingress to be built on Envoy v1.24.1. One notable change is that the team at LightStep and Envoy Maintainers have decided to no longer support the native - *LightStep* tracing driver in favor of using the Open Telemetry driver. The code for LightStep - driver has been completely removed from Envoy code base so Emissary-ingress will no longer - support it either. - The recommended upgrade path is to leverage a supported Tracing driver such as - `Zipkin` and use the [Open Telemetry Collector](https://opentelemetry.io/docs/collector/) to - collect and forward Observabity data to LightStep. + *LightStep* tracing driver in favor of using the Open Telemetry driver. The code for LightStep + driver has been completely removed from Envoy code base so Emissary-ingress will no longer support + it either. + The recommended upgrade path is to leverage a supported Tracing driver such as `Zipkin` + and use the [Open Telemetry Collector](https://opentelemetry.io/docs/collector/) to collect and + forward Observabity data to LightStep. - Feature: /ready endpoint used by emissary is using the admin port (8001 by default). This generates a problem during config reloads with large configs as the admin thread is blocking so diff --git a/charts/emissary-ingress/templates/deployment.yaml b/charts/emissary-ingress/templates/deployment.yaml index 6571eef946..828da4ef3e 100644 --- a/charts/emissary-ingress/templates/deployment.yaml +++ b/charts/emissary-ingress/templates/deployment.yaml @@ -172,12 +172,6 @@ spec: - name: admin containerPort: {{ .Values.adminService.port }} env: - - name: AMBASSADOR_GRPC_METRICS_SINK - value: {{ include "ambassador.fullname" . }}-agent:80 - - name: HOST_IP - valueFrom: - fieldRef: - fieldPath: status.hostIP {{- if .Values.prometheusExporter.enabled }} - name: STATSD_ENABLED value: "true" diff --git a/docs/releaseNotes.yml b/docs/releaseNotes.yml index 23815e973e..96f614a857 100644 --- a/docs/releaseNotes.yml +++ b/docs/releaseNotes.yml @@ -58,6 +58,13 @@ items: This issue was only present in v3alpha1 Mappings. For users who may have this issue, please be sure to re-apply any v3alpha1 Mappings in order to update the stored v2 Mapping and resolve the issue. + - title: Ambassador Agent no longer collects Envoy metrics + type: change + body: >- + When the Ambassador agent is being used, it will no longer attempt to collect and report Envoy metrics. + In previous versions, $productName$ would always create an Envoy stats sink for the agent as long as the AMBASSADOR_GRPC_METRICS_SINK + environmet variable was provided. This environment variable was hardcoded on the release manifests and has now been removed + and an Envoy stats sink for the agent is no longer created. - version: 3.7.2 prevVersion: 3.7.1 @@ -66,7 +73,7 @@ items: - title: Upgrade to Envoy 1.26.4 type: security body: >- - This upgrades $productName$ to be built on Envoy v1.26.4 which includes a security fixes for + This upgrades $productName$ to be built on Envoy v1.26.4 which includes a security fixes for CVE-2023-35942, CVE-2023-35943, VE-2023-35944. - version: 3.7.1 @@ -116,11 +123,11 @@ items: - title: TracingService support for native OpenTelemetry driver type: feature body: >- - In Envoy 1.24, experimental support for a native OpenTelemetry tracing driver - was introduced that allows exporting spans in the otlp format. Many + In Envoy 1.24, experimental support for a native OpenTelemetry tracing driver + was introduced that allows exporting spans in the otlp format. Many Observability platforms accept that format and is the recommend - replacement for the LightStep driver. $productName$ now supports setting the - TracingService.spec.driver=opentelemetry to export spans in + replacement for the LightStep driver. $productName$ now supports setting the + TracingService.spec.driver=opentelemetry to export spans in otlp format.

Thanks to Paul for helping us @@ -135,7 +142,7 @@ items: way emissary was generating Envoy configuration under a single wild-card virtual_host and matching on :authority. - + In v2.Y/v3.Y+, the way emissary generates Envoy configuration changed to address memory pressure and improve route lookup speed in Envoy. However, when including a port in the hostname, an incorrect configuration was generated with an sni match including the port. This has been fixed and the correct envoy configuration is @@ -144,12 +151,12 @@ items: - title: "fix: hostname port issue" link: https://github.com/emissary-ingress/emissary/pull/4816 - - title: Add support for resolving port names in Ingress resource + - title: Add support for resolving port names in Ingress resource type: change body: >- Previously, specifying backend ports by name in Ingress was not supported and would result in defaulting to port 80. This allows emissary-ingress to now resolve port names for backend services. If the port number - cannot be resolved by the name (e.g named port in the Service doesn't exist) then it defaults back + cannot be resolved by the name (e.g named port in the Service doesn't exist) then it defaults back to the original behavior. (Thanks to Anton Ustyuzhanin!). github: @@ -159,13 +166,13 @@ items: - title: Add starupProbe to emissary-apiext server type: change body: >- - The emissary-apiext server is a Kubernetes Conversion Webhook that converts between the + The emissary-apiext server is a Kubernetes Conversion Webhook that converts between the Emissary-ingress CRD versions. On startup, it ensures that a self-signed cert is available - so that K8s API Server can talk to the conversion webhook (*TLS is required by K8s*). We + so that K8s API Server can talk to the conversion webhook (*TLS is required by K8s*). We have introduced a startupProbe to ensure that emissary-apiext server has enough time to - configure the webhooks before running liveness and readiness probes. This is to ensure + configure the webhooks before running liveness and readiness probes. This is to ensure slow startup doesn't cause K8s to needlessly restart the pod. - + - version: 3.4.0 prevVersion: 3.3.0 @@ -198,19 +205,19 @@ items: - `AMBASSADOR_HEALTHCHECK_BIND_ADDRESS`: The address to bind the healthcheck server to. - `AMBASSADOR_HEALTHCHECK_BIND_PORT`: The port to bind the healthcheck server to. - + - `AMBASSADOR_HEALTHCHECK_IP_FAMILY`: The IP family to use for the healthcheck server. - - This allows the healthcheck server to be configured to use IPv6-only k8s environments. + + This allows the healthcheck server to be configured to use IPv6-only k8s environments. (Thanks to Dmitry Golushko!). - title: Upgrade to Envoy 1.24.1 type: feature body: >- - This upgrades $productName$ to be built on Envoy v1.24.1. One notable change is that + This upgrades $productName$ to be built on Envoy v1.24.1. One notable change is that the team at LightStep and Envoy Maintainers have decided to no longer support the - native *LightStep* tracing driver in favor of using the Open Telemetry driver. The code - for LightStep driver has been completely removed from Envoy code base so $productName$ + native *LightStep* tracing driver in favor of using the Open Telemetry driver. The code + for LightStep driver has been completely removed from Envoy code base so $productName$ will no longer support it either. The recommended upgrade path is to leverage a supported Tracing driver such as `Zipkin` diff --git a/manifests/emissary/emissary-defaultns.yaml.in b/manifests/emissary/emissary-defaultns.yaml.in index 0b9a7c4900..ffc58eac63 100644 --- a/manifests/emissary/emissary-defaultns.yaml.in +++ b/manifests/emissary/emissary-defaultns.yaml.in @@ -289,12 +289,6 @@ spec: weight: 100 containers: - env: - - name: AMBASSADOR_GRPC_METRICS_SINK - value: emissary-ingress-agent:80 - - name: HOST_IP - valueFrom: - fieldRef: - fieldPath: status.hostIP - name: AMBASSADOR_NAMESPACE valueFrom: fieldRef: diff --git a/manifests/emissary/emissary-emissaryns.yaml.in b/manifests/emissary/emissary-emissaryns.yaml.in index 04aa7110cc..833390fda0 100644 --- a/manifests/emissary/emissary-emissaryns.yaml.in +++ b/manifests/emissary/emissary-emissaryns.yaml.in @@ -289,12 +289,6 @@ spec: weight: 100 containers: - env: - - name: AMBASSADOR_GRPC_METRICS_SINK - value: emissary-ingress-agent:80 - - name: HOST_IP - valueFrom: - fieldRef: - fieldPath: status.hostIP - name: AMBASSADOR_NAMESPACE valueFrom: fieldRef: diff --git a/python/ambassador/envoy/v3/v3bootstrap.py b/python/ambassador/envoy/v3/v3bootstrap.py index bd80a2dd1d..70c2456260 100644 --- a/python/ambassador/envoy/v3/v3bootstrap.py +++ b/python/ambassador/envoy/v3/v3bootstrap.py @@ -97,60 +97,6 @@ def __init__(self, config: "V3Config") -> None: clusters.append(V3Cluster(config, typecast(IRCluster, log_service.cluster))) stats_sinks = [] - - grpcSink = os.environ.get("AMBASSADOR_GRPC_METRICS_SINK") - if grpcSink: - try: - host, port = split_host_port(grpcSink) - valid = True - except ValueError as ex: - config.ir.logger.error( - "AMBASSADOR_GRPC_METRICS_SINK value %s is invalid: %s" % (grpcSink, ex) - ) - valid = False - - if valid: - stats_sinks.append( - { - "name": "envoy.metrics_service", - "typed_config": { - "@type": "type.googleapis.com/envoy.config.metrics.v3.MetricsServiceConfig", - "transport_api_version": "V3", - "grpc_service": { - "envoy_grpc": {"cluster_name": "envoy_metrics_service"} - }, - }, - } - ) - clusters.append( - { - "name": "envoy_metrics_service", - "type": "strict_dns", - "connect_timeout": "1s", - "http2_protocol_options": {}, - "load_assignment": { - "cluster_name": "envoy_metrics_service", - "endpoints": [ - { - "lb_endpoints": [ - { - "endpoint": { - "address": { - "socket_address": { - "address": host, - "port_value": port, - "protocol": "TCP", - } - } - } - } - ] - } - ], - }, - } - ) - if config.ir.statsd["enabled"]: if config.ir.statsd["dogstatsd"]: name = "envoy.stat_sinks.dog_statsd" diff --git a/python/tests/integration/manifests/ambassador.yaml b/python/tests/integration/manifests/ambassador.yaml index 395f82b2d7..15ef1353bd 100644 --- a/python/tests/integration/manifests/ambassador.yaml +++ b/python/tests/integration/manifests/ambassador.yaml @@ -91,12 +91,6 @@ metadata: spec: containers: - env: - - name: AMBASSADOR_GRPC_METRICS_SINK - value: {self.path.k8s}-agent:80 - - name: HOST_IP - valueFrom: - fieldRef: - fieldPath: status.hostIP - name: AMBASSADOR_NAMESPACE valueFrom: fieldRef: