/test

/test

/test

Here is a more detailed explanation what's going on using a simple HTTP server that gracefully shuts down on SIGTERM and set it up with a readiness probe.

apiVersion: apps/v1
kind: Deployment
metadata:
  name: unready-pod
  labels:
    app: unready
spec:
  replicas: 1
  selector:
    matchLabels:
      app: unready
  template:
    metadata:
      labels:
        app: unready
    spec:
      terminationGracePeriodSeconds: 120
      containers:
      - name: unready
        image: test.jussi/testpod:0.1
        readinessProbe:
          periodSeconds: 1
          httpGet:
            path: /ready
            port: 8080
        livenessProbe:
          exec:
            command:
            - "true"
        ports:
        - containerPort: 8080
---
apiVersion: v1
kind: Service
metadata:
  name: svc-unready
spec:
  selector:
    app: unready
  ports:
    - protocol: TCP
      port: 80
      targetPort: 8080

package main

import (
        "context"
        "fmt"
        "log/slog"
        "net/http"
        "os"
        "os/signal"
        "syscall"
        "time"
)

func main() {
        srv := &http.Server{Addr: ":8080"}
        http.HandleFunc("/sleep", sleeper)
        http.HandleFunc("/ready", func(w http.ResponseWriter, r *http.Request) {
                w.WriteHeader(http.StatusOK)
        })
        slog.Info("Listening on :8080")
        go srv.ListenAndServe()

        sigs := make(chan os.Signal, 1)
        signal.Notify(sigs, syscall.SIGTERM)
        <-sigs
        slog.Info("SIGTERM received, gracefully shutting down")
        ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
        defer cancel()
        srv.Shutdown(ctx)
}

func sleeper(w http.ResponseWriter, r *http.Request) {
        slog.Info("Request received", "method", r.Method, "url", r.URL, "remote", r.RemoteAddr)
        fmt.Fprintf(w, "sleeping 60 seconds...\n")
        w.(http.Flusher).Flush()
        time.Sleep(60 * time.Second)
}

I'm curl'ing the server from a pod in another node:

kubectl run --image=gcr.io/kubernetes-e2e-test-images/agnhost:2.6 --overrides='{"apiVersion": "v1", "spec": {"nodeSelector": { "kubernetes.io/hostname": "kind-control-plane" }}}'  test
time kubectl exec test -- curl --keepalive-time 1 'svc-unready:80/sleep'

When I scale down the deployment to 0 replicas the HTTP server receives a SIGTERM which closes the listening
socket and thus failing the readiness probe. This in turn flips serving to false (evidenced by kubectl get endpointslices/svc-unready-hdt4z -o yaml -w|grep -A3 'conditions:')

Cilium without this will remove the backend from both services and backends maps when serving=false leaving a dangling reference in the conntrack entry and thus results in a RST packet when the TCP keepalive hits it, e.g. curl terminates before 60s with curl: (56) Recv failure: Connection refused.

Cilium with this fix will mark the backend as quarantined as soon as it has serving=false and keeps it in the backend map without changes. This allows the existing connections to continue.

/test

/test

It's a clean implementation, mostly LGTM! The main change in this PR is to exclude backends that are terminating but not serving from load balancing, even if they are the only available backend. I wish Kubernetes integrated such conditions in the service endpoint updates, eliminating the need to add another "terminating-not-serving" state. Couple of follow-ups: (1) Do we need to persist this new state in the BPF LB map? (2) Can you extend the documentation - https://docs.cilium.io/en/latest/network/kubernetes/kubeproxy-free/#graceful-termination?

1. No need to persist anything differently. Essentially the difference to "terminating" is just that we don't pick the backend up as a fallback if no active exist (e.g. won't go into services map).

2. Ah good point! I'll update it with the new information learned during this.

Saw at least one failure around https://github.com/kubernetes/kubernetes/blob/790393ae92e97262827d4f1fba24e8ae65bbada0/test/e2e/network/service.go#L3041C1-L3042C1 in the k8s e2e which seems very suspicious given the changes in this PR. I think we might be implementing the terminating endpoints fallback incorrectly. Though looking at the test I think there might be a chance that it would try to connect before Cilium has processed the EndpointSlice change in which case it wouldn't error out. Will need to dive into this before I dare merging this PR (even though it seems that it is unrelated).

EDIT: Ah looks like we started seeing this failure when we bumped the version: https://github.com/cilium/cilium/actions/runs/16915128150. The failing test case was added already 3 years ago so it's something else that they've changed that started introducing this issue. It could still be we have a bug in how we handle the terminating endpoint fallback, but it's not related to this PR. I'll look into that issue separately, but not have it block this one.

/test

/test

Dropped the needs-backport label for the moment, given #41199