Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EnvoyGateway rateLimitDeployment ignores pod labels #4226

Closed
oscarboher opened this issue Sep 12, 2024 · 6 comments · Fixed by #4228
Closed

EnvoyGateway rateLimitDeployment ignores pod labels #4226

oscarboher opened this issue Sep 12, 2024 · 6 comments · Fixed by #4228
Assignees
Labels
kind/bug Something isn't working provider/kubernetes Issues related to the Kubernetes provider
Milestone

Comments

@oscarboher
Copy link
Contributor

Description:
When configuring the rate limit deployment through the envoy-gateway-config configmap, the provider.kubernetes.rateLimitDeployment.pod.labels field is ignored.
Other fields at the same level like annotations are respected.

Repro steps:

  • gateway v1.1 (experimental channel)

  • envoy-gateway installed through helm chart v0.0.0-latest

  • create GatewayClass with envoy controller.

  • Update envoy-gateway-config configmap in envoy-gateway-system namespace with the following values:

    apiVersion: v1
    kind: ConfigMap
    metadata:
      name: envoy-gateway-config
      namespace: envoy-gateway-system
    data:
      envoy-gateway.yaml: |
        apiVersion: gateway.envoyproxy.io/v1alpha1
        kind: EnvoyGateway
        provider:
          type: Kubernetes
          kubernetes:
            rateLimitDeployment:
              replicas: 4
              pod:
                labels:
                  my-label: test
                annotations:
                  test-anno: test
        gateway:
          controllerName: gateway.envoyproxy.io/gatewayclass-controller
        rateLimit:
          backend:
            type: Redis
            redis:
              url: redis.redis-system.svc.cluster.local:6379
          telemetry:
            metrics:
              prometheus:
                disable: false
            
          
    ```
    
  • Rollout restart envoy-gateway deployment for it to pick up the changes.

  • Deployment envoy-ratelimit has the annotation test-anno: test in the pod spec, but not the label my-label: test

Environment:

  • gateway v1.1 (experimental channel)
  • envoy-gateway installed through helm chart v0.0.0-latest
  • image gw controller: docker.io/envoyproxy/gateway-dev:latest
@zirain
Copy link
Contributor

zirain commented Sep 12, 2024

this indeed a bug, @oscarboher would you like to send a fix?

BTW, I think we may also have issue on merging annotations.

@oscarboher
Copy link
Contributor Author

I wouldn't mind trying! any guidance will be welcome too.
I understand changes should probably go here.

About the merge annotation issue you mean with the prometheus annotations?

@zirain
Copy link
Contributor

zirain commented Sep 12, 2024

that's the point.

@zirain zirain added provider/kubernetes Issues related to the Kubernetes provider and removed triage labels Sep 12, 2024
@arkodg arkodg added kind/bug Something isn't working help wanted Extra attention is needed and removed help wanted Extra attention is needed labels Sep 12, 2024
@arkodg arkodg added this to the v1.2.0-rc1 milestone Sep 12, 2024
@arkodg
Copy link
Contributor

arkodg commented Sep 12, 2024

hey @oscarboher thanks for giving it a try, assigning this you for now, let us know how we can help

@arkodg
Copy link
Contributor

arkodg commented Sep 12, 2024

a workaround could be to use the patch field https://gateway.envoyproxy.io/docs/api/extension_types/#kubernetesdeploymentspec

I haven't tested this config, but something similar should help here

rateLimitDeployment:
  patch:
    template:
      metadata:
        labels:
          my-label: test

@oscarboher
Copy link
Contributor Author

oscarboher commented Sep 12, 2024

running the tests for resource_provider locally, some of the cases in testDeployments fail because they were defining the annotation prometheus.io/scrape: "true"in the test and only that annotation was expected in the deployments test data.
I understand that what is expected is to have all 3 annotations for prometheus if this is not disabled, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working provider/kubernetes Issues related to the Kubernetes provider
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants