Long worker-shutdown-timeout causes long-lived connections to fail

[James-Quigley]

What happened:

The setup:

• gRPC client/server with long lived http2 connections
• worker-shutdown-timeout: 30m
• nginx reloads, starting the shutdown of old workers
• a new version of the grpc server is released, causing all the pods to turn ov

What you expected to happen:
I would expect the workers to gracefully try to terminate the long lived connections, and to continue to route to valid targets in the meantime.

Instead, I find that grpc clients start to get UNAVAILABLE or UNIMPLEMENTED errors, which I presume is from traffic being routed by the old workers to IPs that don't exist anymore (or are assigned to different pods)

NGINX Ingress controller version (exec into the pod and run nginx-ingress-controller --version.):
v1.8.1

Kubernetes version (use kubectl version): 1.27

Environment: EKS

• Cloud provider or hardware configuration: AWS EKS

• OS (e.g. from /etc/os-release): Bottlerocket

• Kernel (e.g. uname -a): 5.15.160

• Install tools: AWS EKS

• Basic cluster related info: v1.27

• How was the ingress-nginx-controller installed:

  • Argo + Helm. Chart version 4.7.1

# ingress-nginx configuration
# https://github.com/kubernetes/ingress-nginx/blob/main/charts/ingress-nginx/values.yaml

ingress-nginx:
  controller:
    image:
      registry: registry.k8s.io
    priorityClassName: cluster-core-services
    kind: Deployment

    autoscaling:
      minReplicas: 9
      maxReplicas: 35
      targetCPUUtilizationPercentage: 65

    maxUnavailable: 2
    resources:
      requests:
        cpu: 3000m
        memory: 4500Mi

    # the reload pod needs to see the nginx pid in the controller pod, so enable sharing of pids
    shareProcessNamespace: true

    # Name of the ingress class to route through this controller
    ingressClassResource:
      name: https-internal
      default: true
      controllerValue: k8s.io/ingress-https-internal

    # Process IngressClass per name
    ingressClassByName: true

    # Use the dedicated ingress nodes
    nodeSelector:
      dedicated: ingress

    # Tolerate taints on dedicated ingress nodes.
    tolerations:
      - key: dedicated
        value: ingress
        operator: Equal
        effect: NoSchedule

    service:
      annotations:
        # Use a (TCP) Network Load Balancer.
        # https://docs.aws.amazon.com/eks/latest/userguide/network-load-balancing.html
        service.beta.kubernetes.io/aws-load-balancer-type: external
        service.beta.kubernetes.io/aws-load-balancer-nlb-target-type: ip
        service.beta.kubernetes.io/aws-load-balancer-target-group-attributes: preserve_client_ip.enabled=true,deregistration_delay.connection_termination.enabled=true,deregistration_delay.timeout_seconds=120
        # "Internal" load balancing is enabled by default.

      externalTrafficPolicy: Local

    metrics:
      enabled: true
      serviceMonitor:
        enabled: true
        namespace: ingress-https-internal
        scrapeInterval: 15s

    ## Additional command line arguments to pass to nginx-ingress-controller
    ## E.g. to specify the default SSL certificate you can use
    ## extraArgs:
    ##   default-ssl-certificate: "<namespace>/<secret_name>"
    extraArgs:
      ingress-class: https-internal
      # Instructs the controller to wait this many seconds before sending the quit signal to nginx.
      # After receiving the quit signal, nginx will attempt to complete any requests before reaching the
      # terminating grace period.
      shutdown-grace-period: 360

    autoscaling:
      enabled: true
      minReplicas: 6
      maxReplicas: 25
      targetCPUUtilizationPercentage: 85
      targetMemoryUtilizationPercentage: null
      behavior:
        scaleDown:
          stabilizationWindowSeconds: 300
          policies:
            - type: Pods
              value: 1
              periodSeconds: 180
        scaleUp:
          stabilizationWindowSeconds: 300
          policies:
            - type: Pods
              value: 2
              periodSeconds: 60

    # see per-env values for resource configuration

    topologySpreadConstraints:
      - maxSkew: 1
        topologyKey: topology.kubernetes.io/zone
        whenUnsatisfiable: DoNotSchedule
        labelSelector:
          matchLabels:
            app.kubernetes.io/instance: ingress-https-internal

    maxUnavailable: 1
    terminationGracePeriodSeconds: 2100  # 35 mins

    affinity:
      podAntiAffinity:
        preferredDuringSchedulingIgnoredDuringExecution:
          - weight: 100
            podAffinityTerm:
              labelSelector:
                matchExpressions:
                  - key: app.kubernetes.io/instance
                    operator: In
                    values:
                      - ingress-https-internal
              topologyKey: kubernetes.io/hostname

    allowSnippetAnnotations: true
    config:
      use-forwarded-headers: true
      worker-shutdown-timeout: 30m
      enable-underscores-in-headers: true

  admissionWebhooks:
    patch:
      image:
        registry: registry.k8s.io

How to reproduce this issue:
Still working on an easy way to reproduce this

Anything else we need to know:
https://github.com/kubernetes/ingress-nginx/blob/main/rootfs/etc/nginx/lua/balancer.lua#L297-L317
Seems to sync the list of endpoints periodically, based on the configuration that gets handled by the lua http server (https://github.com/kubernetes/ingress-nginx/blob/main/rootfs/etc/nginx/lua/configuration.lua#L245-L248)
That value is sent from the Go controller here: https://github.com/kubernetes/ingress-nginx/blob/main/internal/ingress/controller/nginx.go#L944-L987

The timer in the worker init though:
https://github.com/openresty/lua-nginx-module?tab=readme-ov-file#ngxtimerevery

timer will be created every delay seconds until the current Nginx worker process starts exiting
timer expiration happens when the Nginx worker process is trying to shut down, as in an Nginx configuration reload triggered by the HUP signal or in an Nginx server shutdown

Therefore I believe that if:

• worker-shutdown-timeout is long
• an nginx reload is triggered
• during the duration of worker-shutdown-timeout, the backing pod ips change (therefore the endpoints change)
• and a client is holding open a connection and sending requests
• Those requests will be routed to invalid targets, as the worker will no longer be getting an updated list of endpoints.

[k8s-ci-robot]

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[James-Quigley]

I'm also happy to share some extra evidence privately. But I was able to:

• use prometheus to pull a list of IPs for pods before and after the deployment rollout
• query ingress-nginx logs for requests to the service in question, filtering for IPs in the first list, but omitting requests to ips in the second list

The log volume shows a massive drop off in requests right as the rollout happens (which I would expect). However, a small tail of requests continues to the old pods, despite the fact that they don't exist (or at least aren't used by the app in question anymore)

[longwuyuan]

/remove-kind bug
/triage needs-information

I think that the user her #11508 also reports the same behavior..

While we wait for other comments and maybe some expert opinion, I made an assumption that in a multi-replica use-case, the routing and loadbalancing of traffic will be the same regardless of the backend-protocol from controller to backend being gRPC or HTTP. Let me know if my assumption is wrong. It will take me too long to dive into the code and ascertain this.

So I tested with a 5 replica deployment using image nginx:alpine and generating at least 1 new connection per second with multiple sessions, to that deployment's HTTP backend-protocol ingress.

I could not reproduce the problem indicated in these 2 issues. I did see a transition related response code though.

So I am inclined to think that off-beat connection requesting client needs to handle the transitions better. It seems like such client have a myopic view that neither they use affinity/persistence nor do they accommodate the transition related events like graceful-drain of connections and route to another backend pod.

In any case, it will help a lot to get a step-by-step guide to reproduce the problem.

[github-actions]

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.

[alexnovak]

Following up on this issue - I work on the same environment as @James-Quigley . We've learned a few additional details since this issue was submitted.

As far as I can tell experimentally - the issue seems to be that the set of available next hops are set when the request is submitted, and do not update for the request on retries.

In our environment, this can manifest as UNIMPLEMENTED if the number of retries is large enough, as nginx may issue an old request to a new GRPC server that happens to inherit the same IP as its now dead previous recipient. The UNAVAILABLE response seems to come when we hit our retry cap, but what's concerning about the UNAVAILABLE response is that it can happen while other healthy pods for a service are present.

I'd expect that the retry requests would refresh the pool of available hosts between attempts, but I've seen some evidence that suggests that's not the case.

My experimental setup is:

• Two instances of a grpc service
• A grpc service with the following annotations

nginx.ingress.kubernetes.io/backend-protocol: grpcs
nginx.ingress.kubernetes.io/proxy-body-size: 250m
nginx.ingress.kubernetes.io/proxy-buffer-size: 16k
nginx.ingress.kubernetes.io/proxy-buffering: "on"
nginx.ingress.kubernetes.io/proxy-buffers-number: "16"

• A client that can spam requests to the grpc backend. I've had the most success recreating this with streaming requests, but that might just be because they take longer for our particular service under test.

After getting the client to start firing, I'll then terminate the two service pods, and their replacements a couple times giving a one to two second delay (this stream request can take several seconds to several minutes to respond).

Without a cap on requests, we've observed that these requests can loop until either the client terminates the connection, or the ingress-nginx pod terminates. In our ingress-nginx error logs, nginx reattempts the connection every 60 seconds, but each connection is for previously held IP addresses for this service.

I've also run a variant of this experiment, where only one of the service pods is killed. In this case all requests eventually succeed.

Top of mind - I can think of two situations where this can potentially cause issues for clients:

• During deploys, where a large number of pods may change. If all (or even some) pods in the pool associated with your request terminate, and then your request has a failure, we can run into this scenario.
• During long-lived requests, pods may naturally migrate. With a long enough request, and bad luck, when your request fails, you may have no more living pods in the pool when you retry.

Happy to receive any corrections on this if I'm way off-base. Would appreciate any assistance in getting to the bottom of this.

[longwuyuan]

• The initial issue-description needs simplification to remove any vague hint that the ingress-nginx-controller pods have events like termination/recreation/scaling etc
• All the instrumentation and specs in & around the K8S object of "kind: Pod" owned by a object of "kind: Deployment" are aimed at having the best possible chance "physically". For example see this

% k explain deployment.spec.strategy 
GROUP:      apps
KIND:       Deployment
VERSION:    v1

FIELD: strategy <DeploymentStrategy>


DESCRIPTION:
    The deployment strategy to use to replace existing pods with new ones.
    DeploymentStrategy describes how to replace existing pods with new ones.
    
FIELDS:
  rollingUpdate <RollingUpdateDeployment>
    Rolling update config params. Present only if DeploymentStrategyType =
    RollingUpdate.

  type  <string>
  enum: Recreate, RollingUpdate
    Type of deployment. Can be "Recreate" or "RollingUpdate". Default is
    RollingUpdate.
    
    Possible enum values:
     - `"Recreate"` Kill all existing pods before creating new ones.
     - `"RollingUpdate"` Replace the old ReplicaSets by new one using rolling
    update i.e gradually scale down the old ReplicaSets and scale up the new
    one.

Context here is are you seeking test results of Recreate strategy or RollingUpdate strategy. This is about the specific testing step of deleting both pods of a backend service

[alexnovak]

Thanks for your prompt response @longwuyuan!

Can you expand on what you're looking for in the first point? I'm happy to speak to James and request that we edit the description, but I'm not sure he's referring to those events at the pod-level. I think he's instead referring to nginx's behavior when respawning child worker threads on a config reload. These don't (typically) scale, but they do terminate and recreate with some regularity, depending on how often ingress resources are modified.

For your second point - I agree that my experiment setup is contrived - but that's because I'm trying to understand a particular behavior about ingress-nginx. We see a large number of UNAVAILABLE and UNIMPLEMENTED requests when a larger service performs a rolling update. It seems like that could be because the pool of ip addresses for a request is set on the receipt of the request, and doesn't receive updates for retries. If this is an intended behavior of the system, then we can try to alter our deployment strategy to work around that, but I haven't come across any documentation for this behavior in my search. Is this something you know top of mind?

Thanks again for your help here, I really appreciate your guidance.