Skip to content

Commit

Permalink
fix: rateLimitDeployment ignoring pod labels and annotation merge (en…
Browse files Browse the repository at this point in the history
…voyproxy#4228)

* fix labels and annotation merges for rate limit deployment

Signed-off-by: Oscar Boher <[email protected]>

* fix tests and label merge

Signed-off-by: Oscar Boher <[email protected]>

* fix annotation merge if prometheus was disabled and annotations were defined

Signed-off-by: Oscar Boher <[email protected]>

* renamed labels and annotations to specify they apply to pods only

Signed-off-by: Oscar Boher <[email protected]>

* linter

Signed-off-by: Oscar Boher <[email protected]>

* fix resource provider tests to new annotation behavior

Signed-off-by: Oscar Boher <[email protected]>

* go linter

Signed-off-by: Oscar Boher <[email protected]>

* fix gen-check

Signed-off-by: Oscar Boher <[email protected]>

* pod labels selector comment

Signed-off-by: Oscar Boher <[email protected]>

---------

Signed-off-by: Oscar Boher <[email protected]>
Co-authored-by: zirain <[email protected]>
Co-authored-by: Arko Dasgupta <[email protected]>
  • Loading branch information
3 people authored Oct 1, 2024
1 parent db1f437 commit cf84927
Show file tree
Hide file tree
Showing 11 changed files with 391 additions and 8 deletions.
28 changes: 20 additions & 8 deletions internal/infrastructure/kubernetes/ratelimit/resource_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
_ "embed"
"strconv"

"golang.org/x/exp/maps"
appsv1 "k8s.io/api/apps/v1"
autoscalingv2 "k8s.io/api/autoscaling/v2"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -196,19 +197,30 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) {
}

containers := expectedRateLimitContainers(r.rateLimit, r.rateLimitDeployment, r.Namespace)
labels := rateLimitLabels()
selector := resource.GetSelector(labels)
selector := resource.GetSelector(rateLimitLabels())

podLabels := rateLimitLabels()
if r.rateLimitDeployment.Pod.Labels != nil {
maps.Copy(podLabels, r.rateLimitDeployment.Pod.Labels)
// Copy overwrites values in the dest map if they exist in the src map https://pkg.go.dev/maps#Copy
// It's applied again with the rateLimitLabels that are used as deployment selector to ensure those are not overwritten by user input
maps.Copy(podLabels, rateLimitLabels())
}

var annotations map[string]string
var podAnnotations map[string]string
if enablePrometheus(r.rateLimit) {
annotations = map[string]string{
podAnnotations = map[string]string{
"prometheus.io/path": "/metrics",
"prometheus.io/port": strconv.Itoa(PrometheusPort),
"prometheus.io/scrape": "true",
}
}
if r.rateLimitDeployment.Pod.Annotations != nil {
annotations = r.rateLimitDeployment.Pod.Annotations
if podAnnotations != nil {
maps.Copy(podAnnotations, r.rateLimitDeployment.Pod.Annotations)
} else {
podAnnotations = r.rateLimitDeployment.Pod.Annotations
}
}

deployment := &appsv1.Deployment{
Expand All @@ -218,16 +230,16 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) {
},
ObjectMeta: metav1.ObjectMeta{
Namespace: r.Namespace,
Labels: labels,
Labels: rateLimitLabels(),
},
Spec: appsv1.DeploymentSpec{
Replicas: r.rateLimitDeployment.Replicas,
Strategy: *r.rateLimitDeployment.Strategy,
Selector: selector,
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: selector.MatchLabels,
Annotations: annotations,
Labels: podLabels,
Annotations: podAnnotations,
},
Spec: corev1.PodSpec{
Containers: containers,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"flag"
"fmt"
"os"
"strconv"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -678,6 +679,50 @@ func TestDeployment(t *testing.T) {
},
},
},
{
caseName: "merge-labels",
rateLimit: &egv1a1.RateLimit{
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
},
},
},
deploy: &egv1a1.KubernetesDeploymentSpec{
Pod: &egv1a1.KubernetesPodSpec{
Labels: map[string]string{
"app.kubernetes.io/name": InfraName,
"app.kubernetes.io/component": "ratelimit",
"app.kubernetes.io/managed-by": "envoy-gateway",
"key1": "value1",
"key2": "value2",
},
},
},
},
{
caseName: "merge-annotations",
rateLimit: &egv1a1.RateLimit{
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
},
},
},
deploy: &egv1a1.KubernetesDeploymentSpec{
Pod: &egv1a1.KubernetesPodSpec{
Annotations: map[string]string{
"prometheus.io/path": "/metrics",
"prometheus.io/port": strconv.Itoa(PrometheusPort),
"prometheus.io/scrape": "true",
"key1": "value1",
"key2": "value2",
},
},
},
},
}
for _, tc := range cases {
t.Run(tc.caseName, func(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ spec:
template:
metadata:
annotations:
prometheus.io/path: /metrics
prometheus.io/port: "19001"
prometheus.io/scrape: "true"
creationTimestamp: null
labels:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ spec:
template:
metadata:
annotations:
prometheus.io/path: /metrics
prometheus.io/port: "19001"
prometheus.io/scrape: "true"
creationTimestamp: null
labels:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ spec:
template:
metadata:
annotations:
prometheus.io/path: /metrics
prometheus.io/port: "19001"
prometheus.io/scrape: "true"
creationTimestamp: null
labels:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
apiVersion: apps/v1
kind: Deployment
metadata:
creationTimestamp: null
labels:
app.kubernetes.io/component: ratelimit
app.kubernetes.io/managed-by: envoy-gateway
app.kubernetes.io/name: envoy-ratelimit
name: envoy-ratelimit
namespace: envoy-gateway-system
ownerReferences:
- apiVersion: apps/v1
kind: Deployment
name: envoy-gateway
uid: test-owner-reference-uid-for-deployment
spec:
progressDeadlineSeconds: 600
revisionHistoryLimit: 10
selector:
matchLabels:
app.kubernetes.io/component: ratelimit
app.kubernetes.io/managed-by: envoy-gateway
app.kubernetes.io/name: envoy-ratelimit
strategy:
type: RollingUpdate
template:
metadata:
annotations:
key1: value1
key2: value2
prometheus.io/path: /metrics
prometheus.io/port: "19001"
prometheus.io/scrape: "true"
creationTimestamp: null
labels:
app.kubernetes.io/component: ratelimit
app.kubernetes.io/managed-by: envoy-gateway
app.kubernetes.io/name: envoy-ratelimit
spec:
automountServiceAccountToken: false
containers:
- command:
- /bin/ratelimit
env:
- name: RUNTIME_ROOT
value: /data
- name: RUNTIME_SUBDIRECTORY
value: ratelimit
- name: RUNTIME_IGNOREDOTFILES
value: "true"
- name: RUNTIME_WATCH_ROOT
value: "false"
- name: LOG_LEVEL
value: info
- name: USE_STATSD
value: "false"
- name: CONFIG_TYPE
value: GRPC_XDS_SOTW
- name: CONFIG_GRPC_XDS_SERVER_URL
value: envoy-gateway:18001
- name: CONFIG_GRPC_XDS_NODE_ID
value: envoy-ratelimit
- name: GRPC_SERVER_USE_TLS
value: "true"
- name: GRPC_SERVER_TLS_CERT
value: /certs/tls.crt
- name: GRPC_SERVER_TLS_KEY
value: /certs/tls.key
- name: GRPC_SERVER_TLS_CA_CERT
value: /certs/ca.crt
- name: CONFIG_GRPC_XDS_SERVER_USE_TLS
value: "true"
- name: CONFIG_GRPC_XDS_CLIENT_TLS_CERT
value: /certs/tls.crt
- name: CONFIG_GRPC_XDS_CLIENT_TLS_KEY
value: /certs/tls.key
- name: CONFIG_GRPC_XDS_SERVER_TLS_CACERT
value: /certs/ca.crt
- name: FORCE_START_WITHOUT_INITIAL_CONFIG
value: "true"
- name: REDIS_SOCKET_TYPE
value: tcp
- name: REDIS_URL
value: redis.redis.svc:6379
- name: USE_PROMETHEUS
value: "true"
- name: PROMETHEUS_ADDR
value: :19001
- name: PROMETHEUS_MAPPER_YAML
value: /etc/statsd-exporter/conf.yaml
image: envoyproxy/ratelimit:master
imagePullPolicy: IfNotPresent
name: envoy-ratelimit
ports:
- containerPort: 8081
name: grpc
protocol: TCP
readinessProbe:
failureThreshold: 1
httpGet:
path: /healthcheck
port: 8080
scheme: HTTP
periodSeconds: 5
successThreshold: 1
timeoutSeconds: 1
resources:
requests:
cpu: 100m
memory: 512Mi
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop:
- ALL
privileged: false
readOnlyRootFilesystem: true
runAsGroup: 65534
runAsNonRoot: true
runAsUser: 65534
seccompProfile:
type: RuntimeDefault
startupProbe:
failureThreshold: 30
httpGet:
path: /healthcheck
port: 8080
scheme: HTTP
periodSeconds: 10
successThreshold: 1
timeoutSeconds: 1
terminationMessagePath: /dev/termination-log
terminationMessagePolicy: File
volumeMounts:
- mountPath: /certs
name: certs
readOnly: true
- mountPath: /etc/statsd-exporter
name: statsd-exporter-config
readOnly: true
dnsPolicy: ClusterFirst
restartPolicy: Always
schedulerName: default-scheduler
serviceAccountName: envoy-ratelimit
terminationGracePeriodSeconds: 300
volumes:
- name: certs
secret:
defaultMode: 420
secretName: envoy-rate-limit
- configMap:
defaultMode: 420
name: statsd-exporter-config
optional: true
name: statsd-exporter-config
status: {}
Loading

0 comments on commit cf84927

Please sign in to comment.