From 362dcc71fa68a69127446b85acd2eb3f8559db8d Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Thu, 8 Feb 2024 16:00:07 -0500 Subject: [PATCH 01/10] First metrics pass --- .../templates/crd-gatewayclassconfigs-v1.yaml | 345 +++++++++--------- .../templates/gateway-resources-job.yaml | 9 + charts/consul/values.yaml | 13 + control-plane/api-gateway/binding/binder.go | 6 +- .../api-gateway/binding/registration.go | 20 +- .../api-gateway/common/helm_config.go | 10 + control-plane/api-gateway/common/metrics.go | 98 +++++ .../controllers/gateway_controller.go | 1 + .../api-gateway/gatekeeper/dataplane.go | 23 +- .../api-gateway/gatekeeper/deployment.go | 22 +- .../api/v1alpha1/api_gateway_types.go | 21 ++ .../api/v1alpha1/zz_generated.deepcopy.go | 31 ++ ...sul.hashicorp.com_gatewayclassconfigs.yaml | 19 + .../subcommand/gateway-resources/command.go | 60 +++ .../inject-connect/v1controllers.go | 33 +- 15 files changed, 517 insertions(+), 194 deletions(-) create mode 100644 control-plane/api-gateway/common/metrics.go diff --git a/charts/consul/templates/crd-gatewayclassconfigs-v1.yaml b/charts/consul/templates/crd-gatewayclassconfigs-v1.yaml index 685dbf3617..714e4e2b0c 100644 --- a/charts/consul/templates/crd-gatewayclassconfigs-v1.yaml +++ b/charts/consul/templates/crd-gatewayclassconfigs-v1.yaml @@ -20,182 +20,201 @@ spec: singular: gatewayclassconfig scope: Cluster versions: - - name: v1alpha1 - schema: - openAPIV3Schema: - description: GatewayClassConfig defines the values that may be set on a GatewayClass - for Consul API Gateway. - properties: - apiVersion: - description: 'APIVersion defines the versioned schema of this representation + - name: v1alpha1 + schema: + openAPIV3Schema: + description: GatewayClassConfig defines the values that may be set on a GatewayClass + for Consul API Gateway. + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' - type: string - kind: - description: 'Kind is a string value representing the REST resource this + type: string + kind: + description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' - type: string - metadata: - type: object - spec: - description: Spec defines the desired state of GatewayClassConfig. - properties: - copyAnnotations: - description: Annotation Information to copy to services or deployments - properties: - service: - description: List of annotations to copy to the gateway service. - items: - type: string - type: array - type: object - deployment: - description: Deployment defines the deployment configuration for the - gateway. - properties: - defaultInstances: - default: 1 - description: Number of gateway instances that should be deployed - by default - format: int32 - maximum: 8 - minimum: 1 - type: integer - maxInstances: - default: 8 - description: Max allowed number of gateway instances - format: int32 - maximum: 8 - minimum: 1 - type: integer - minInstances: - default: 1 - description: Minimum allowed number of gateway instances - format: int32 - maximum: 8 - minimum: 1 - type: integer - resources: - description: Resources defines the resource requirements for the - gateway. - properties: - claims: - description: "Claims lists the names of resources, defined + type: string + metadata: + type: object + spec: + description: Spec defines the desired state of GatewayClassConfig. + properties: + copyAnnotations: + description: Annotation Information to copy to services or deployments + properties: + service: + description: List of annotations to copy to the gateway service. + items: + type: string + type: array + type: object + deployment: + description: Deployment defines the deployment configuration for the + gateway. + properties: + defaultInstances: + default: 1 + description: Number of gateway instances that should be deployed + by default + format: int32 + maximum: 8 + minimum: 1 + type: integer + maxInstances: + default: 8 + description: Max allowed number of gateway instances + format: int32 + maximum: 8 + minimum: 1 + type: integer + minInstances: + default: 1 + description: Minimum allowed number of gateway instances + format: int32 + maximum: 8 + minimum: 1 + type: integer + resources: + description: Resources defines the resource requirements for the + gateway. + properties: + claims: + description: "Claims lists the names of resources, defined in spec.resourceClaims, that are used by this container. \n This is an alpha field and requires enabling the DynamicResourceAllocation feature gate. \n This field is immutable. It can only be set for containers." - items: - description: ResourceClaim references one entry in PodSpec.ResourceClaims. - properties: - name: - description: Name must match the name of one entry in - pod.spec.resourceClaims of the Pod where this field - is used. It makes that resource available inside a - container. - type: string - required: - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map - limits: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'Limits describes the maximum amount of compute - resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + items: + description: ResourceClaim references one entry in PodSpec.ResourceClaims. + properties: + name: + description: Name must match the name of one entry in + pod.spec.resourceClaims of the Pod where this field + is used. It makes that resource available inside a + container. + type: string + required: + - name type: object - requests: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'Requests describes the minimum amount of compute + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute + resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute resources required. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' - type: object - type: object - type: object - mapPrivilegedContainerPorts: - description: The value to add to privileged ports ( ports < 1024) - for gateway containers - format: int32 - type: integer - nodeSelector: - additionalProperties: + type: object + type: object + type: object + mapPrivilegedContainerPorts: + description: The value to add to privileged ports ( ports < 1024) + for gateway containers + format: int32 + type: integer + metrics: + description: Metrics defines how to configure the metrics for a gateway. + properties: + enabled: + description: Enable metrics for this class of gateways. If unspecified, + will inherit behavior from the global Helm configuration. + type: boolean + path: + default: /metrics + description: The path used for metrics. type: string - description: 'NodeSelector is a selector which must be true for the + port: + default: 20200 + description: The port used for metrics. + format: int32 + maximum: 65535 + minimum: 1024 + type: integer + type: object + nodeSelector: + additionalProperties: + type: string + description: 'NodeSelector is a selector which must be true for the pod to fit on a node. Selector which must match a node''s labels for the pod to be scheduled on that node. More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/' - type: object - openshiftSCCName: - description: The name of the OpenShift SecurityContextConstraints - resource for this gateway class to use. - type: string - podSecurityPolicy: - description: The name of an existing Kubernetes PodSecurityPolicy - to bind to the managed ServiceAccount if ACLs are managed. - type: string - serviceType: - description: Service Type string describes ingress methods for a service - enum: - - ClusterIP - - NodePort - - LoadBalancer - type: string - tolerations: - description: 'Tolerations allow the scheduler to schedule nodes with + type: object + openshiftSCCName: + description: The name of the OpenShift SecurityContextConstraints + resource for this gateway class to use. + type: string + podSecurityPolicy: + description: The name of an existing Kubernetes PodSecurityPolicy + to bind to the managed ServiceAccount if ACLs are managed. + type: string + serviceType: + description: Service Type string describes ingress methods for a service + enum: + - ClusterIP + - NodePort + - LoadBalancer + type: string + tolerations: + description: 'Tolerations allow the scheduler to schedule nodes with matching taints. More Info: https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/' - items: - description: The pod this Toleration is attached to tolerates any - taint that matches the triple using the matching - operator . - properties: - effect: - description: Effect indicates the taint effect to match. Empty - means match all taint effects. When specified, allowed values - are NoSchedule, PreferNoSchedule and NoExecute. - type: string - key: - description: Key is the taint key that the toleration applies - to. Empty means match all taint keys. If the key is empty, - operator must be Exists; this combination means to match all - values and all keys. - type: string - operator: - description: Operator represents a key's relationship to the - value. Valid operators are Exists and Equal. Defaults to Equal. - Exists is equivalent to wildcard for value, so that a pod - can tolerate all taints of a particular category. - type: string - tolerationSeconds: - description: TolerationSeconds represents the period of time - the toleration (which must be of effect NoExecute, otherwise - this field is ignored) tolerates the taint. By default, it - is not set, which means tolerate the taint forever (do not - evict). Zero and negative values will be treated as 0 (evict - immediately) by the system. - format: int64 - type: integer - value: - description: Value is the taint value the toleration matches - to. If the operator is Exists, the value should be empty, - otherwise just a regular string. - type: string - type: object - type: array - type: object - type: object - served: true - storage: true - {{- end }} \ No newline at end of file + items: + description: The pod this Toleration is attached to tolerates any + taint that matches the triple using the matching + operator . + properties: + effect: + description: Effect indicates the taint effect to match. Empty + means match all taint effects. When specified, allowed values + are NoSchedule, PreferNoSchedule and NoExecute. + type: string + key: + description: Key is the taint key that the toleration applies + to. Empty means match all taint keys. If the key is empty, + operator must be Exists; this combination means to match all + values and all keys. + type: string + operator: + description: Operator represents a key's relationship to the + value. Valid operators are Exists and Equal. Defaults to Equal. + Exists is equivalent to wildcard for value, so that a pod + can tolerate all taints of a particular category. + type: string + tolerationSeconds: + description: TolerationSeconds represents the period of time + the toleration (which must be of effect NoExecute, otherwise + this field is ignored) tolerates the taint. By default, it + is not set, which means tolerate the taint forever (do not + evict). Zero and negative values will be treated as 0 (evict + immediately) by the system. + format: int64 + type: integer + value: + description: Value is the taint value the toleration matches + to. If the operator is Exists, the value should be empty, + otherwise just a regular string. + type: string + type: object + type: array + type: object + type: object + served: true + storage: true +{{- end }} diff --git a/charts/consul/templates/gateway-resources-job.yaml b/charts/consul/templates/gateway-resources-job.yaml index 5934372ed3..1653458009 100644 --- a/charts/consul/templates/gateway-resources-job.yaml +++ b/charts/consul/templates/gateway-resources-job.yaml @@ -102,6 +102,15 @@ spec: {{- end }} - -map-privileged-container-ports={{ .Values.connectInject.apiGateway.managedGatewayClass.mapPrivilegedContainerPorts }} {{- end}} + {{- if .Values.connectInject.apiGateway.managedGatewayClass.metrics.enabled }} + - -enable-metrics={{ .Values.connectInject.apiGateway.managedGatewayClass.metrics.enabled }} + {{- end }} + {{- if .Values.connectInject.apiGateway.managedGatewayClass.metrics.path }} + - -metrics-path={{ .Values.connectInject.apiGateway.managedGatewayClass.metrics.path }} + {{- end }} + {{- if .Values.connectInject.apiGateway.managedGatewayClass.metrics.port }} + - -metrics-port={{ .Values.connectInject.apiGateway.managedGatewayClass.metrics.port }} + {{- end }} resources: requests: memory: "50Mi" diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 49d986cd7d..f6574de199 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -3314,6 +3314,19 @@ apiGateway: # @type: string service: null + # Metrics settings for gateways created with this gateway class configuration. + metrics: + # This value enables or disables metrics collection on a gateway, overriding the global gateway metrics collection settings. + # @type: boolean + enabled: null + # This value sets the port to use for scraping gateway metrics via prometheus, defaults to 20200 if not set. Must be in the port + # range of 1024-65535. + # @type: int + port: null + # This value sets the path to use for scraping gateway metrics via prometheus, defaults to /metrics if not set. + # @type: string + path: null + # This value defines the number of pods to deploy for each Gateway as well as a min and max number of pods for all Gateways # # Example: diff --git a/control-plane/api-gateway/binding/binder.go b/control-plane/api-gateway/binding/binder.go index c6557985b5..c704a6b04e 100644 --- a/control-plane/api-gateway/binding/binder.go +++ b/control-plane/api-gateway/binding/binder.go @@ -67,6 +67,9 @@ type BinderConfig struct { // Policies is a list containing all GatewayPolicies that are part of the Gateway Deployment Policies []v1alpha1.GatewayPolicy + + // Configuration from helm. + HelmConfig common.HelmConfig } // Binder is used for generating a Snapshot of all operations that should occur both @@ -199,7 +202,8 @@ func (b *Binder) Snapshot() *Snapshot { OnUpdate: b.handleGatewaySyncStatus(snapshot, &b.config.Gateway, consulStatus), }) - registrations := registrationsForPods(entry.Namespace, b.config.Gateway, registrationPods) + metricsConfig := common.GatewayMetricsConfig(b.config.Gateway, *gatewayClassConfig, b.config.HelmConfig) + registrations := registrationsForPods(metricsConfig, entry.Namespace, b.config.Gateway, registrationPods) snapshot.Consul.Registrations = registrations // deregister any not explicitly registered service diff --git a/control-plane/api-gateway/binding/registration.go b/control-plane/api-gateway/binding/registration.go index ae26ab51f6..a31988e508 100644 --- a/control-plane/api-gateway/binding/registration.go +++ b/control-plane/api-gateway/binding/registration.go @@ -6,6 +6,7 @@ package binding import ( "fmt" + gatewaycommon "github.com/hashicorp/consul-k8s/control-plane/api-gateway/common" "github.com/hashicorp/consul-k8s/control-plane/connect-inject/common" "github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants" "github.com/hashicorp/consul/api" @@ -22,22 +23,34 @@ const ( // consulKubernetesCheckName is the name of health check in Consul for Kubernetes readiness status. consulKubernetesCheckName = "Kubernetes Readiness Check" + + // metricsConfiguration is the configuration key for binding a prometheus port to the envoy instance + metricsConfiguration = "envoy_prometheus_bind_addr" ) -func registrationsForPods(namespace string, gateway gwv1beta1.Gateway, pods []corev1.Pod) []api.CatalogRegistration { +func registrationsForPods(metrics gatewaycommon.MetricsConfig, namespace string, gateway gwv1beta1.Gateway, pods []corev1.Pod) []api.CatalogRegistration { registrations := []api.CatalogRegistration{} for _, pod := range pods { - registrations = append(registrations, registrationForPod(namespace, gateway, pod)) + registrations = append(registrations, registrationForPod(metrics, namespace, gateway, pod)) } return registrations } -func registrationForPod(namespace string, gateway gwv1beta1.Gateway, pod corev1.Pod) api.CatalogRegistration { +func registrationForPod(metrics gatewaycommon.MetricsConfig, namespace string, gateway gwv1beta1.Gateway, pod corev1.Pod) api.CatalogRegistration { healthStatus := api.HealthCritical if isPodReady(pod) { healthStatus = api.HealthPassing } + var proxyConfigOverrides *api.AgentServiceConnectProxyConfig + if metrics.Enabled { + proxyConfigOverrides = &api.AgentServiceConnectProxyConfig{ + Config: map[string]interface{}{ + metricsConfiguration: fmt.Sprintf("%s:%d", pod.Status.PodIP, metrics.Port), + }, + } + } + return api.CatalogRegistration{ Node: common.ConsulNodeNameFromK8sNode(pod.Spec.NodeName), Address: pod.Status.HostIP, @@ -50,6 +63,7 @@ func registrationForPod(namespace string, gateway gwv1beta1.Gateway, pod corev1. Service: gateway.Name, Address: pod.Status.PodIP, Namespace: namespace, + Proxy: proxyConfigOverrides, Meta: map[string]string{ constants.MetaKeyPodName: pod.Name, constants.MetaKeyKubeNS: pod.Namespace, diff --git a/control-plane/api-gateway/common/helm_config.go b/control-plane/api-gateway/common/helm_config.go index 7ce8e0778a..5ee208b887 100644 --- a/control-plane/api-gateway/common/helm_config.go +++ b/control-plane/api-gateway/common/helm_config.go @@ -40,6 +40,16 @@ type HelmConfig struct { // MapPrivilegedServicePorts is the value which Consul will add to privileged container port values (ports < 1024) // defined on a Gateway. MapPrivilegedServicePorts int + + // EnableGatewayMetrics indicates whether or not gateway metrics should be enabled + // by default on a deployed gateway, passed from the helm chart via command-line flags to our controller. + EnableGatewayMetrics bool + + // The default path to use for scraping prometheus metrics, passed from the helm chart via command-line flags to our controller. + DefaultPrometheusScrapePath string + + // The default port to use for scraping prometheus metrics, passed from the helm chart via command-line flags to our controller. + DefaultPrometheusScrapePort string } type ConsulConfig struct { diff --git a/control-plane/api-gateway/common/metrics.go b/control-plane/api-gateway/common/metrics.go new file mode 100644 index 0000000000..248cbe737e --- /dev/null +++ b/control-plane/api-gateway/common/metrics.go @@ -0,0 +1,98 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package common + +import ( + "strconv" + + "github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1" + "github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants" + gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" +) + +const ( + defaultScrapePort = 20200 + defaultScrapePath = "/metrics" +) + +type MetricsConfig struct { + Enabled bool + Path string + Port int +} + +func gatewayMetricsEnabled(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig, config HelmConfig) bool { + // first check our annotations, if something is there, then it means we've explicitly + // annotated metrics collection + if scrape, isSet := gateway.Annotations[constants.AnnotationEnableMetrics]; isSet { + return scrape == "true" + } + + // if it's not set on the annotation, then we check to see if it's set on the GatewayClassConfig + if gcc.Spec.MetricsSpec.Enabled != nil { + return *gcc.Spec.MetricsSpec.Enabled + } + + // otherwise, fallback to the global helm setting + return config.EnableGatewayMetrics +} + +func fetchPortString(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig, config HelmConfig) string { + // first check our annotations, if something is there, then it means we've explicitly + // annotated metrics collection + if portString, isSet := gateway.Annotations[constants.AnnotationPrometheusScrapePort]; isSet { + return portString + } + + // if it's not set on the annotation, then we check to see if it's set on the GatewayClassConfig + if gcc.Spec.MetricsSpec.Port != nil { + return strconv.Itoa(int(*gcc.Spec.MetricsSpec.Port)) + } + + // otherwise, fallback to the global helm setting + return config.DefaultPrometheusScrapePort +} + +func gatewayMetricsPort(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig, config HelmConfig) int { + portString := fetchPortString(gateway, gcc, config) + + port, err := strconv.Atoi(portString) + if err != nil { + // if we can't parse the port string, just use the default + // TODO: log an error + return defaultScrapePort + } + + if port < 1024 || port > 65535 { + // if we requested a privileged port, use the default + // TODO: log an error + return defaultScrapePort + } + + return port +} + +func gatewayMetricsPath(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig, config HelmConfig) string { + // first check our annotations, if something is there, then it means we've explicitly + // annotated metrics collection + if path, isSet := gateway.Annotations[constants.AnnotationPrometheusScrapePath]; isSet { + return path + } + + // if it's not set on the annotation, then we check to see if it's set on the GatewayClassConfig + if gcc.Spec.MetricsSpec.Path != nil { + return *gcc.Spec.MetricsSpec.Path + } + + // otherwise, fallback to the global helm setting + return config.DefaultPrometheusScrapePath +} + +func GatewayMetricsConfig(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig, config common.HelmConfig) metricsConfig { + return MetricsConfig{ + Enabled: gatewayMetricsEnabled(gateway, gcc, config), + Path: gatewayMetricsPath(gateway, gcc, config), + Port: gatewayMetricsPort(gateway, gcc, config), + } +} diff --git a/control-plane/api-gateway/controllers/gateway_controller.go b/control-plane/api-gateway/controllers/gateway_controller.go index 8447e69f64..537100fd70 100644 --- a/control-plane/api-gateway/controllers/gateway_controller.go +++ b/control-plane/api-gateway/controllers/gateway_controller.go @@ -204,6 +204,7 @@ func (r *GatewayController) Reconcile(ctx context.Context, req ctrl.Request) (ct ConsulGateway: consulGateway, ConsulGatewayServices: consulServices, Policies: policies, + HelmConfig: r.HelmConfig, }) updates := binder.Snapshot() diff --git a/control-plane/api-gateway/gatekeeper/dataplane.go b/control-plane/api-gateway/gatekeeper/dataplane.go index 090464e9c8..16839cbb09 100644 --- a/control-plane/api-gateway/gatekeeper/dataplane.go +++ b/control-plane/api-gateway/gatekeeper/dataplane.go @@ -22,12 +22,11 @@ const ( netBindCapability = "NET_BIND_SERVICE" consulDataplaneDNSBindHost = "127.0.0.1" consulDataplaneDNSBindPort = 8600 - defaultPrometheusScrapePath = "/metrics" defaultEnvoyProxyConcurrency = 1 volumeName = "consul-connect-inject-data" ) -func consulDataplaneContainer(config common.HelmConfig, gcc v1alpha1.GatewayClassConfig, name, namespace string) (corev1.Container, error) { +func consulDataplaneContainer(metrics common.MetricsConfig, config common.HelmConfig, gcc v1alpha1.GatewayClassConfig, name, namespace string) (corev1.Container, error) { // Extract the service account token's volume mount. var ( err error @@ -38,7 +37,7 @@ func consulDataplaneContainer(config common.HelmConfig, gcc v1alpha1.GatewayClas bearerTokenFile = "/var/run/secrets/kubernetes.io/serviceaccount/token" } - args, err := getDataplaneArgs(namespace, config, bearerTokenFile, name) + args, err := getDataplaneArgs(metrics, namespace, config, bearerTokenFile, name) if err != nil { return corev1.Container{}, err } @@ -100,6 +99,15 @@ func consulDataplaneContainer(config common.HelmConfig, gcc v1alpha1.GatewayClas Name: "proxy-health", ContainerPort: int32(constants.ProxyDefaultHealthPort), }) + + if metrics.Enabled { + container.Ports = append(container.Ports, corev1.ContainerPort{ + Name: "prometheus", + ContainerPort: int32(metrics.Port), + Protocol: corev1.ProtocolTCP, + }) + } + // Configure the resource requests and limits for the proxy if they are set. if gcc.Spec.DeploymentSpec.Resources != nil { container.Resources = *gcc.Spec.DeploymentSpec.Resources @@ -122,7 +130,7 @@ func consulDataplaneContainer(config common.HelmConfig, gcc v1alpha1.GatewayClas return container, nil } -func getDataplaneArgs(namespace string, config common.HelmConfig, bearerTokenFile string, name string) ([]string, error) { +func getDataplaneArgs(metrics common.MetricsConfig, namespace string, config common.HelmConfig, bearerTokenFile string, name string) ([]string, error) { proxyIDFileName := "/consul/connect-inject/proxyid" envoyConcurrency := defaultEnvoyProxyConcurrency @@ -170,9 +178,10 @@ func getDataplaneArgs(namespace string, config common.HelmConfig, bearerTokenFil args = append(args, fmt.Sprintf("-envoy-admin-bind-port=%d", 19000)) - // Set a default scrape path that can be overwritten by the annotation. - prometheusScrapePath := defaultPrometheusScrapePath - args = append(args, "-telemetry-prom-scrape-path="+prometheusScrapePath) + if metrics.Enabled { + // Set up metrics collection. + args = append(args, "-telemetry-prom-scrape-path="+metrics.Path) + } return args, nil } diff --git a/control-plane/api-gateway/gatekeeper/deployment.go b/control-plane/api-gateway/gatekeeper/deployment.go index f3e9545e57..c0a191a7ce 100644 --- a/control-plane/api-gateway/gatekeeper/deployment.go +++ b/control-plane/api-gateway/gatekeeper/deployment.go @@ -5,9 +5,11 @@ package gatekeeper import ( "context" + "strconv" "github.com/hashicorp/consul-k8s/control-plane/api-gateway/common" "github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1" + "github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants" "k8s.io/apimachinery/pkg/types" appsv1 "k8s.io/api/apps/v1" @@ -90,7 +92,19 @@ func (g *Gatekeeper) deployment(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayC return nil, err } - container, err := consulDataplaneContainer(config, gcc, gateway.Name, gateway.Namespace) + annotations := map[string]string{ + "consul.hashicorp.com/connect-inject": "false", + } + + metrics := common.GatewayMetricsConfig(gateway, gcc, config) + + if metrics.enabled { + annotations[constants.AnnotationPrometheusScrape] = "true" + annotations[constants.AnnotationPrometheusPath] = metrics.path + annotations[constants.AnnotationPrometheusPort] = strconv.Itoa(metrics.port) + } + + container, err := consulDataplaneContainer(metrics, config, gcc, gateway.Name, gateway.Namespace) if err != nil { return nil, err } @@ -108,10 +122,8 @@ func (g *Gatekeeper) deployment(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayC }, Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ - Labels: common.LabelsForGateway(&gateway), - Annotations: map[string]string{ - "consul.hashicorp.com/connect-inject": "false", - }, + Labels: common.LabelsForGateway(&gateway), + Annotations: annotations, }, Spec: corev1.PodSpec{ Volumes: []corev1.Volume{ diff --git a/control-plane/api/v1alpha1/api_gateway_types.go b/control-plane/api/v1alpha1/api_gateway_types.go index 90f6376d98..15a083ad1b 100644 --- a/control-plane/api/v1alpha1/api_gateway_types.go +++ b/control-plane/api/v1alpha1/api_gateway_types.go @@ -65,6 +65,9 @@ type GatewayClassConfigSpec struct { // The value to add to privileged ports ( ports < 1024) for gateway containers MapPrivilegedContainerPorts int32 `json:"mapPrivilegedContainerPorts,omitempty"` + + // Metrics defines how to configure the metrics for a gateway. + MetricsSpec MetricsSpec `json:"metrics,omitempty"` } // +k8s:deepcopy-gen=true @@ -90,6 +93,24 @@ type DeploymentSpec struct { Resources *corev1.ResourceRequirements `json:"resources,omitempty"` } +// +k8s:deepcopy-gen=true + +type MetricsSpec struct { + // +kubebuilder:default:=20200 + // +kubebuilder:validation:Maximum=65535 + // +kubebuilder:validation:Minimum=1024 + // The port used for metrics. + Port *int32 `json:"port,omitempty"` + + // +kubebuilder:default:=/metrics + // The path used for metrics. + Path *string `json:"path,omitempty"` + + // Enable metrics for this class of gateways. If unspecified, will inherit + // behavior from the global Helm configuration. + Enabled *bool `json:"enabled,omitempty"` +} + //+kubebuilder:object:generate=true // CopyAnnotationsSpec defines the annotations that should be copied to the gateway service. diff --git a/control-plane/api/v1alpha1/zz_generated.deepcopy.go b/control-plane/api/v1alpha1/zz_generated.deepcopy.go index 045bd5144e..865a0d90fe 100644 --- a/control-plane/api/v1alpha1/zz_generated.deepcopy.go +++ b/control-plane/api/v1alpha1/zz_generated.deepcopy.go @@ -555,6 +555,7 @@ func (in *GatewayClassConfigSpec) DeepCopyInto(out *GatewayClassConfigSpec) { } in.DeploymentSpec.DeepCopyInto(&out.DeploymentSpec) in.CopyAnnotations.DeepCopyInto(&out.CopyAnnotations) + in.MetricsSpec.DeepCopyInto(&out.MetricsSpec) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GatewayClassConfigSpec. @@ -1977,6 +1978,36 @@ func (in *MeshTLSConfig) DeepCopy() *MeshTLSConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MetricsSpec) DeepCopyInto(out *MetricsSpec) { + *out = *in + if in.Port != nil { + in, out := &in.Port, &out.Port + *out = new(int32) + **out = **in + } + if in.Path != nil { + in, out := &in.Path, &out.Path + *out = new(string) + **out = **in + } + if in.Enabled != nil { + in, out := &in.Enabled, &out.Enabled + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MetricsSpec. +func (in *MetricsSpec) DeepCopy() *MetricsSpec { + if in == nil { + return nil + } + out := new(MetricsSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PassiveHealthCheck) DeepCopyInto(out *PassiveHealthCheck) { *out = *in diff --git a/control-plane/config/crd/bases/consul.hashicorp.com_gatewayclassconfigs.yaml b/control-plane/config/crd/bases/consul.hashicorp.com_gatewayclassconfigs.yaml index ff3158f2a7..6b0cc06f36 100644 --- a/control-plane/config/crd/bases/consul.hashicorp.com_gatewayclassconfigs.yaml +++ b/control-plane/config/crd/bases/consul.hashicorp.com_gatewayclassconfigs.yaml @@ -127,6 +127,25 @@ spec: for gateway containers format: int32 type: integer + metrics: + description: Metrics defines how to configure the metrics for a gateway. + properties: + enabled: + description: Enable metrics for this class of gateways. If unspecified, + will inherit behavior from the global Helm configuration. + type: boolean + path: + default: /metrics + description: The path used for metrics. + type: string + port: + default: 20200 + description: The port used for metrics. + format: int32 + maximum: 65535 + minimum: 1024 + type: integer + type: object nodeSelector: additionalProperties: type: string diff --git a/control-plane/subcommand/gateway-resources/command.go b/control-plane/subcommand/gateway-resources/command.go index 3fb7dd9c24..c903b6ea9a 100644 --- a/control-plane/subcommand/gateway-resources/command.go +++ b/control-plane/subcommand/gateway-resources/command.go @@ -11,6 +11,7 @@ import ( "fmt" "io" "os" + "strconv" "sync" "time" @@ -94,6 +95,10 @@ type Command struct { flagMapPrivilegedContainerPorts int + flagEnableMetrics string + flagMetricsPort string + flagMetricsPath string + k8sClient client.Client once sync.Once @@ -160,6 +165,10 @@ func (c *Command) init() { "gateway container.", ) + c.flags.StringVar(&c.flagEnableMetrics, "enable-metrics", "", "specify as 'true' or 'false' to enable or disable metrics collection") + c.flags.StringVar(&c.flagMetricsPath, "metrics-path", "", "specify to set the path used for metrics scraping") + c.flags.StringVar(&c.flagMetricsPort, "metrics-port", "", "specify to set the port used for metrics scraping") + c.flags.StringVar(&c.flagGatewayConfigLocation, "gateway-config-file-location", gatewayConfigFilename, "specify a different location for where the gateway config file is") @@ -278,6 +287,16 @@ func (c *Command) Run(args []string) int { }, } + if metricsEnabled, isSet := getMetricsEnabled(c.flagEnableMetrics); isSet { + class.Spec.MetricsConfig.Enabled = metricsEnabled + if port, isValid := getScrapePort(c.flagMetricsPort); isValid { + class.Spec.MetricsConfig.Port = port + } + if path, isSet := getScrapePath(c.flagMetricsPath); isSet { + class.Spec.MetricsConfig.Path = path + } + } + if err := forceV1ClassConfig(context.Background(), c.k8sClient, classConfig); err != nil { c.UI.Error(err.Error()) return 1 @@ -346,6 +365,18 @@ func (c *Command) validateFlags() error { } } + if c.flagEnableMetrics != "" { + if _, valid := getMetricsEnabled(c.flagEnableMetrics); !valid { + return errors.New("-enable-metrics must be either 'true' or 'false'") + } + } + + if c.flagMetricsPort != "" { + if _, valid := getScrapePort(c.flagMetricsPort); !valid { + return errors.New("-metrics-port must be a valid unprivileged port number") + } + } + return nil } @@ -560,6 +591,35 @@ func exponentialBackoffWithMaxIntervalAndTime() *backoff.ExponentialBackOff { return backoff } +func getScrapePort(v string) (int, bool) { + port, err := strconv.Atoi(v) + if err != nil { + // we only use the port if it's actually valid + return 0, false + } + if port < 1024 || port > 65535 { + return 0, false + } + return port, true +} + +func getScrapePath(v string) (string, bool) { + if v == "" { + return "", false + } + return v, true +} + +func getMetricsEnabled(v string) (bool, bool) { + if v == "true" { + return true, true + } + if v == "false" { + return false, true + } + return false, false +} + func nonZeroOrNil(v int) *int32 { if v == 0 { return nil diff --git a/control-plane/subcommand/inject-connect/v1controllers.go b/control-plane/subcommand/inject-connect/v1controllers.go index c407c99ccf..1a44395b0d 100644 --- a/control-plane/subcommand/inject-connect/v1controllers.go +++ b/control-plane/subcommand/inject-connect/v1controllers.go @@ -113,21 +113,24 @@ func (c *Command) configureV1Controllers(ctx context.Context, mgr manager.Manage HTTPPort: consulConfig.HTTPPort, APITimeout: consulConfig.APITimeout, }, - ImageDataplane: c.flagConsulDataplaneImage, - ImageConsulK8S: c.flagConsulK8sImage, - ConsulDestinationNamespace: c.flagConsulDestinationNamespace, - NamespaceMirroringPrefix: c.flagK8SNSMirroringPrefix, - EnableNamespaces: c.flagEnableNamespaces, - PeeringEnabled: c.flagEnablePeering, - EnableOpenShift: c.flagEnableOpenShift, - EnableNamespaceMirroring: c.flagEnableK8SNSMirroring, - AuthMethod: c.consul.ConsulLogin.AuthMethod, - LogLevel: c.flagLogLevel, - LogJSON: c.flagLogJSON, - TLSEnabled: c.consul.UseTLS, - ConsulTLSServerName: c.consul.TLSServerName, - ConsulPartition: c.consul.Partition, - ConsulCACert: string(c.caCertPem), + ImageDataplane: c.flagConsulDataplaneImage, + ImageConsulK8S: c.flagConsulK8sImage, + ConsulDestinationNamespace: c.flagConsulDestinationNamespace, + NamespaceMirroringPrefix: c.flagK8SNSMirroringPrefix, + EnableNamespaces: c.flagEnableNamespaces, + PeeringEnabled: c.flagEnablePeering, + EnableOpenShift: c.flagEnableOpenShift, + EnableNamespaceMirroring: c.flagEnableK8SNSMirroring, + AuthMethod: c.consul.ConsulLogin.AuthMethod, + LogLevel: c.flagLogLevel, + LogJSON: c.flagLogJSON, + TLSEnabled: c.consul.UseTLS, + ConsulTLSServerName: c.consul.TLSServerName, + ConsulPartition: c.consul.Partition, + ConsulCACert: string(c.caCertPem), + EnableGatewayMetrics: c.flagEnableGatewayMetrics, + DefaultPrometheusScrapePath: c.flagDefaultPrometheusScrapePath, + DefaultPrometheusScrapePort: c.flagDefaultPrometheusScrapePort, }, AllowK8sNamespacesSet: allowK8sNamespaces, DenyK8sNamespacesSet: denyK8sNamespaces, From 8ec939106d4ab01d8a3fa6b3e60184316a0d16bd Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Thu, 8 Feb 2024 16:27:53 -0500 Subject: [PATCH 02/10] Fix up build --- control-plane/api-gateway/common/metrics.go | 14 ++++++------- .../api-gateway/gatekeeper/deployment.go | 6 +++--- .../api/v1alpha1/api_gateway_types.go | 2 +- .../api/v1alpha1/zz_generated.deepcopy.go | 2 +- .../subcommand/gateway-resources/command.go | 21 ++++++++++--------- 5 files changed, 23 insertions(+), 22 deletions(-) diff --git a/control-plane/api-gateway/common/metrics.go b/control-plane/api-gateway/common/metrics.go index 248cbe737e..dc927f739d 100644 --- a/control-plane/api-gateway/common/metrics.go +++ b/control-plane/api-gateway/common/metrics.go @@ -30,8 +30,8 @@ func gatewayMetricsEnabled(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassC } // if it's not set on the annotation, then we check to see if it's set on the GatewayClassConfig - if gcc.Spec.MetricsSpec.Enabled != nil { - return *gcc.Spec.MetricsSpec.Enabled + if gcc.Spec.Metrics.Enabled != nil { + return *gcc.Spec.Metrics.Enabled } // otherwise, fallback to the global helm setting @@ -46,8 +46,8 @@ func fetchPortString(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig, } // if it's not set on the annotation, then we check to see if it's set on the GatewayClassConfig - if gcc.Spec.MetricsSpec.Port != nil { - return strconv.Itoa(int(*gcc.Spec.MetricsSpec.Port)) + if gcc.Spec.Metrics.Port != nil { + return strconv.Itoa(int(*gcc.Spec.Metrics.Port)) } // otherwise, fallback to the global helm setting @@ -81,15 +81,15 @@ func gatewayMetricsPath(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConf } // if it's not set on the annotation, then we check to see if it's set on the GatewayClassConfig - if gcc.Spec.MetricsSpec.Path != nil { - return *gcc.Spec.MetricsSpec.Path + if gcc.Spec.Metrics.Path != nil { + return *gcc.Spec.Metrics.Path } // otherwise, fallback to the global helm setting return config.DefaultPrometheusScrapePath } -func GatewayMetricsConfig(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig, config common.HelmConfig) metricsConfig { +func GatewayMetricsConfig(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig, config HelmConfig) MetricsConfig { return MetricsConfig{ Enabled: gatewayMetricsEnabled(gateway, gcc, config), Path: gatewayMetricsPath(gateway, gcc, config), diff --git a/control-plane/api-gateway/gatekeeper/deployment.go b/control-plane/api-gateway/gatekeeper/deployment.go index c0a191a7ce..225bbacddb 100644 --- a/control-plane/api-gateway/gatekeeper/deployment.go +++ b/control-plane/api-gateway/gatekeeper/deployment.go @@ -98,10 +98,10 @@ func (g *Gatekeeper) deployment(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayC metrics := common.GatewayMetricsConfig(gateway, gcc, config) - if metrics.enabled { + if metrics.Enabled { annotations[constants.AnnotationPrometheusScrape] = "true" - annotations[constants.AnnotationPrometheusPath] = metrics.path - annotations[constants.AnnotationPrometheusPort] = strconv.Itoa(metrics.port) + annotations[constants.AnnotationPrometheusPath] = metrics.Path + annotations[constants.AnnotationPrometheusPort] = strconv.Itoa(metrics.Port) } container, err := consulDataplaneContainer(metrics, config, gcc, gateway.Name, gateway.Namespace) diff --git a/control-plane/api/v1alpha1/api_gateway_types.go b/control-plane/api/v1alpha1/api_gateway_types.go index 15a083ad1b..2cb76708af 100644 --- a/control-plane/api/v1alpha1/api_gateway_types.go +++ b/control-plane/api/v1alpha1/api_gateway_types.go @@ -67,7 +67,7 @@ type GatewayClassConfigSpec struct { MapPrivilegedContainerPorts int32 `json:"mapPrivilegedContainerPorts,omitempty"` // Metrics defines how to configure the metrics for a gateway. - MetricsSpec MetricsSpec `json:"metrics,omitempty"` + Metrics MetricsSpec `json:"metrics,omitempty"` } // +k8s:deepcopy-gen=true diff --git a/control-plane/api/v1alpha1/zz_generated.deepcopy.go b/control-plane/api/v1alpha1/zz_generated.deepcopy.go index 865a0d90fe..da89af6777 100644 --- a/control-plane/api/v1alpha1/zz_generated.deepcopy.go +++ b/control-plane/api/v1alpha1/zz_generated.deepcopy.go @@ -555,7 +555,7 @@ func (in *GatewayClassConfigSpec) DeepCopyInto(out *GatewayClassConfigSpec) { } in.DeploymentSpec.DeepCopyInto(&out.DeploymentSpec) in.CopyAnnotations.DeepCopyInto(&out.CopyAnnotations) - in.MetricsSpec.DeepCopyInto(&out.MetricsSpec) + in.Metrics.DeepCopyInto(&out.Metrics) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GatewayClassConfigSpec. diff --git a/control-plane/subcommand/gateway-resources/command.go b/control-plane/subcommand/gateway-resources/command.go index c903b6ea9a..9b1a9345b3 100644 --- a/control-plane/subcommand/gateway-resources/command.go +++ b/control-plane/subcommand/gateway-resources/command.go @@ -275,6 +275,17 @@ func (c *Command) Run(args []string) int { }, } + if metricsEnabled, isSet := getMetricsEnabled(c.flagEnableMetrics); isSet { + classConfig.Spec.Metrics.Enabled = &metricsEnabled + if port, isValid := getScrapePort(c.flagMetricsPort); isValid { + port32 := int32(port) + classConfig.Spec.Metrics.Port = &port32 + } + if path, isSet := getScrapePath(c.flagMetricsPath); isSet { + classConfig.Spec.Metrics.Path = &path + } + } + class := &gwv1beta1.GatewayClass{ ObjectMeta: metav1.ObjectMeta{Name: c.flagGatewayClassName, Labels: labels}, Spec: gwv1beta1.GatewayClassSpec{ @@ -287,16 +298,6 @@ func (c *Command) Run(args []string) int { }, } - if metricsEnabled, isSet := getMetricsEnabled(c.flagEnableMetrics); isSet { - class.Spec.MetricsConfig.Enabled = metricsEnabled - if port, isValid := getScrapePort(c.flagMetricsPort); isValid { - class.Spec.MetricsConfig.Port = port - } - if path, isSet := getScrapePath(c.flagMetricsPath); isSet { - class.Spec.MetricsConfig.Path = path - } - } - if err := forceV1ClassConfig(context.Background(), c.k8sClient, classConfig); err != nil { c.UI.Error(err.Error()) return 1 From e9e51501b3cfe222b7632a1eee65f4e227de1ba5 Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Thu, 8 Feb 2024 16:57:08 -0500 Subject: [PATCH 03/10] move to non-deprecated chart options --- charts/consul/values.yaml | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index f6574de199..8cfc97511b 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -2244,6 +2244,19 @@ connectInject: # @type: string service: null + # Metrics settings for gateways created with this gateway class configuration. + metrics: + # This value enables or disables metrics collection on a gateway, overriding the global gateway metrics collection settings. + # @type: boolean + enabled: null + # This value sets the port to use for scraping gateway metrics via prometheus, defaults to 20200 if not set. Must be in the port + # range of 1024-65535. + # @type: int + port: null + # This value sets the path to use for scraping gateway metrics via prometheus, defaults to /metrics if not set. + # @type: string + path: null + # The resource settings for Pods handling traffic for Gateway API. # @recurse: false # @type: map @@ -3314,19 +3327,6 @@ apiGateway: # @type: string service: null - # Metrics settings for gateways created with this gateway class configuration. - metrics: - # This value enables or disables metrics collection on a gateway, overriding the global gateway metrics collection settings. - # @type: boolean - enabled: null - # This value sets the port to use for scraping gateway metrics via prometheus, defaults to 20200 if not set. Must be in the port - # range of 1024-65535. - # @type: int - port: null - # This value sets the path to use for scraping gateway metrics via prometheus, defaults to /metrics if not set. - # @type: string - path: null - # This value defines the number of pods to deploy for each Gateway as well as a min and max number of pods for all Gateways # # Example: From 53bdf97f783e271cf51851fb3dc77ece38f51f61 Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Thu, 8 Feb 2024 17:22:02 -0500 Subject: [PATCH 04/10] Fix up charts and defaults --- charts/consul/templates/crd-gatewayclassconfigs-v1.yaml | 2 -- charts/consul/templates/gateway-resources-job.yaml | 4 ++-- charts/consul/values.yaml | 2 +- control-plane/api/v1alpha1/api_gateway_types.go | 2 -- .../crd/bases/consul.hashicorp.com_gatewayclassconfigs.yaml | 2 -- 5 files changed, 3 insertions(+), 9 deletions(-) diff --git a/charts/consul/templates/crd-gatewayclassconfigs-v1.yaml b/charts/consul/templates/crd-gatewayclassconfigs-v1.yaml index 714e4e2b0c..41023c19dc 100644 --- a/charts/consul/templates/crd-gatewayclassconfigs-v1.yaml +++ b/charts/consul/templates/crd-gatewayclassconfigs-v1.yaml @@ -139,11 +139,9 @@ spec: will inherit behavior from the global Helm configuration. type: boolean path: - default: /metrics description: The path used for metrics. type: string port: - default: 20200 description: The port used for metrics. format: int32 maximum: 65535 diff --git a/charts/consul/templates/gateway-resources-job.yaml b/charts/consul/templates/gateway-resources-job.yaml index 1653458009..7b278d10b3 100644 --- a/charts/consul/templates/gateway-resources-job.yaml +++ b/charts/consul/templates/gateway-resources-job.yaml @@ -102,8 +102,8 @@ spec: {{- end }} - -map-privileged-container-ports={{ .Values.connectInject.apiGateway.managedGatewayClass.mapPrivilegedContainerPorts }} {{- end}} - {{- if .Values.connectInject.apiGateway.managedGatewayClass.metrics.enabled }} - - -enable-metrics={{ .Values.connectInject.apiGateway.managedGatewayClass.metrics.enabled }} + {{- if (ne (.Values.connectInject.apiGateway.managedGatewayClass.metrics.enabled | toString) "-") }} + - -enable-metrics={{ .Values.connectInject.apiGateway.managedGatewayClass.metrics.enabled | toString }} {{- end }} {{- if .Values.connectInject.apiGateway.managedGatewayClass.metrics.path }} - -metrics-path={{ .Values.connectInject.apiGateway.managedGatewayClass.metrics.path }} diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 8cfc97511b..e5114a9ed8 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -2248,7 +2248,7 @@ connectInject: metrics: # This value enables or disables metrics collection on a gateway, overriding the global gateway metrics collection settings. # @type: boolean - enabled: null + enabled: "-" # This value sets the port to use for scraping gateway metrics via prometheus, defaults to 20200 if not set. Must be in the port # range of 1024-65535. # @type: int diff --git a/control-plane/api/v1alpha1/api_gateway_types.go b/control-plane/api/v1alpha1/api_gateway_types.go index 2cb76708af..bc7b65fbe4 100644 --- a/control-plane/api/v1alpha1/api_gateway_types.go +++ b/control-plane/api/v1alpha1/api_gateway_types.go @@ -96,13 +96,11 @@ type DeploymentSpec struct { // +k8s:deepcopy-gen=true type MetricsSpec struct { - // +kubebuilder:default:=20200 // +kubebuilder:validation:Maximum=65535 // +kubebuilder:validation:Minimum=1024 // The port used for metrics. Port *int32 `json:"port,omitempty"` - // +kubebuilder:default:=/metrics // The path used for metrics. Path *string `json:"path,omitempty"` diff --git a/control-plane/config/crd/bases/consul.hashicorp.com_gatewayclassconfigs.yaml b/control-plane/config/crd/bases/consul.hashicorp.com_gatewayclassconfigs.yaml index 6b0cc06f36..c2a857db34 100644 --- a/control-plane/config/crd/bases/consul.hashicorp.com_gatewayclassconfigs.yaml +++ b/control-plane/config/crd/bases/consul.hashicorp.com_gatewayclassconfigs.yaml @@ -135,11 +135,9 @@ spec: will inherit behavior from the global Helm configuration. type: boolean path: - default: /metrics description: The path used for metrics. type: string port: - default: 20200 description: The port used for metrics. format: int32 maximum: 65535 From 76880fbdd7b32afcea3aff6bc5c4a6aa5dfcb7df Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Tue, 26 Mar 2024 10:17:57 -0400 Subject: [PATCH 05/10] Add changelog --- .changelog/3811.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/3811.txt diff --git a/.changelog/3811.txt b/.changelog/3811.txt new file mode 100644 index 0000000000..e333a9df84 --- /dev/null +++ b/.changelog/3811.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +api-gateway: Expose prometheus scrape metrics on api-gateway pods. +``` From efe0384a55f46231bf107b2d7f51edcd4dbd7db8 Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Tue, 26 Mar 2024 10:22:48 -0400 Subject: [PATCH 06/10] Fix bad merge --- control-plane/api-gateway/gatekeeper/deployment.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/control-plane/api-gateway/gatekeeper/deployment.go b/control-plane/api-gateway/gatekeeper/deployment.go index ea4fa226b8..d3bb56a69f 100644 --- a/control-plane/api-gateway/gatekeeper/deployment.go +++ b/control-plane/api-gateway/gatekeeper/deployment.go @@ -94,7 +94,9 @@ func (g *Gatekeeper) deployment(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayC } annotations := map[string]string{ - "consul.hashicorp.com/connect-inject": "false", + "consul.hashicorp.com/connect-inject": "false", + constants.AnnotationGatewayConsulServiceName: gateway.Name, + constants.AnnotationGatewayKind: "api-gateway", } metrics := common.GatewayMetricsConfig(gateway, gcc, config) @@ -123,17 +125,8 @@ func (g *Gatekeeper) deployment(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayC }, Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ -<<<<<<< HEAD Labels: common.LabelsForGateway(&gateway), Annotations: annotations, -======= - Labels: common.LabelsForGateway(&gateway), - Annotations: map[string]string{ - constants.AnnotationInject: "false", - constants.AnnotationGatewayConsulServiceName: gateway.Name, - constants.AnnotationGatewayKind: "api-gateway", - }, ->>>>>>> cc5850e4b2d9b1d132a442517b939b3266ee0c4d }, Spec: corev1.PodSpec{ Volumes: []corev1.Volume{ From 6c09cf1a8b26e6eebf7587f01c5dea41f75b18f2 Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Tue, 26 Mar 2024 10:29:12 -0400 Subject: [PATCH 07/10] Fix test --- control-plane/api-gateway/binding/registration_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/control-plane/api-gateway/binding/registration_test.go b/control-plane/api-gateway/binding/registration_test.go index 356915f9f7..6f60257112 100644 --- a/control-plane/api-gateway/binding/registration_test.go +++ b/control-plane/api-gateway/binding/registration_test.go @@ -6,6 +6,7 @@ package binding import ( "testing" + "github.com/hashicorp/consul-k8s/control-plane/api-gateway/common" "github.com/hashicorp/consul/api" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -68,7 +69,7 @@ func TestRegistrationsForPods_Health(t *testing.T) { }, } { t.Run(name, func(t *testing.T) { - registrations := registrationsForPods(tt.consulNamespace, tt.gateway, tt.pods) + registrations := registrationsForPods(common.MetricsConfig{}, tt.consulNamespace, tt.gateway, tt.pods) require.Len(t, registrations, len(tt.expected)) for i := range registrations { From 10aad7348392e03bd57b4b71cc88dd71085405b5 Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Tue, 26 Mar 2024 10:35:07 -0400 Subject: [PATCH 08/10] fix linter error --- control-plane/api-gateway/binding/registration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/control-plane/api-gateway/binding/registration.go b/control-plane/api-gateway/binding/registration.go index a31988e508..489e765c61 100644 --- a/control-plane/api-gateway/binding/registration.go +++ b/control-plane/api-gateway/binding/registration.go @@ -24,7 +24,7 @@ const ( // consulKubernetesCheckName is the name of health check in Consul for Kubernetes readiness status. consulKubernetesCheckName = "Kubernetes Readiness Check" - // metricsConfiguration is the configuration key for binding a prometheus port to the envoy instance + // metricsConfiguration is the configuration key for binding a prometheus port to the envoy instance. metricsConfiguration = "envoy_prometheus_bind_addr" ) From 6e7790bba746b5ded8f3c02d9e585054b85ea563 Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Tue, 26 Mar 2024 10:50:32 -0400 Subject: [PATCH 09/10] Fix extra yaml block from bad merge --- charts/consul/templates/gateway-resources-job.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/charts/consul/templates/gateway-resources-job.yaml b/charts/consul/templates/gateway-resources-job.yaml index 584fbcca18..e43efc8a9a 100644 --- a/charts/consul/templates/gateway-resources-job.yaml +++ b/charts/consul/templates/gateway-resources-job.yaml @@ -79,7 +79,6 @@ spec: - -openshift-scc-name={{ .Values.connectInject.apiGateway.managedGatewayClass.openshiftSCCName }} {{- end }} - -map-privileged-container-ports={{ .Values.connectInject.apiGateway.managedGatewayClass.mapPrivilegedContainerPorts }} - {{- end}} {{- if (ne (.Values.connectInject.apiGateway.managedGatewayClass.metrics.enabled | toString) "-") }} - -enable-metrics={{ .Values.connectInject.apiGateway.managedGatewayClass.metrics.enabled | toString }} {{- end }} From 8a096a6756596cde8002face23497028eef76233 Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Tue, 26 Mar 2024 12:43:26 -0400 Subject: [PATCH 10/10] Switch == true check to use ParseBool --- control-plane/api-gateway/common/metrics.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/control-plane/api-gateway/common/metrics.go b/control-plane/api-gateway/common/metrics.go index dc927f739d..8ba582c8a5 100644 --- a/control-plane/api-gateway/common/metrics.go +++ b/control-plane/api-gateway/common/metrics.go @@ -26,7 +26,12 @@ func gatewayMetricsEnabled(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassC // first check our annotations, if something is there, then it means we've explicitly // annotated metrics collection if scrape, isSet := gateway.Annotations[constants.AnnotationEnableMetrics]; isSet { - return scrape == "true" + enabled, err := strconv.ParseBool(scrape) + if err == nil { + return enabled + } + // TODO: log an error + // we fall through to the other metrics enabled checks } // if it's not set on the annotation, then we check to see if it's set on the GatewayClassConfig