Skip to content

Commit 57d7008

Browse files
Metrics Server: report NotReady when metrics service gRPC connection is down (#8226)
* Metrics Server: report NotReady when metrics service gRPC connection is down The keda-operator-metrics-apiserver (the adapter) kept reporting Kubernetes-Ready even when its gRPC connection to keda-operator:9666 was down for a long time. Because the pod stayed Ready, the external metrics APIService kept routing HPA external-metric requests to that replica, where every request failed. Add a readiness check named 'metricsservice-grpc-connection' that reports NotReady whenever the gRPC connection to the metrics service is not in the Ready state. A replica that cannot establish or re-establish the connection is then removed from the APIService endpoints, so requests are routed only to replicas that can serve them. To support the check without blocking, add a non-blocking GrpcClient.GetConnectionState() that returns the underlying connection's connectivity.State, and have makeProvider return the gRPC client so main can register the check via the apiserver's AddReadyzChecks. Fixes #8211 Signed-off-by: avneetbansal-aws <284363899+avneetbansal-aws@users.noreply.github.com> * Metrics Server: reconnect idle gRPC connection from readiness check The readiness check calls GrpcClient.GetConnectionState(), which only read the connection state. Once a non-Ready state removed the replica from the APIService endpoints, no RPCs flowed through the client, so nothing would wake a connection that gRPC parks in Idle after its idle timeout (30m by default). The replica could then stay NotReady permanently. GetConnectionState now triggers a non-blocking Connect() when it observes an Idle connection, mirroring WaitForConnectionReady, so the connection leaves Idle and can recover without any RPC. Adds TestGetConnectionStateReconnectsWhenIdle covering this. Signed-off-by: avneetbansal-aws <284363899+avneetbansal-aws@users.noreply.github.com> --------- Signed-off-by: avneetbansal-aws <284363899+avneetbansal-aws@users.noreply.github.com> Co-authored-by: avneetbansal-aws <284363899+avneetbansal-aws@users.noreply.github.com> Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
1 parent cb1ce09 commit 57d7008

3 files changed

Lines changed: 137 additions & 8 deletions

File tree

‎cmd/adapter/main.go‎

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,10 @@ import (
2727
"github.com/prometheus/client_golang/prometheus"
2828
"github.com/prometheus/client_golang/prometheus/collectors"
2929
"go.uber.org/zap/zapcore"
30+
"google.golang.org/grpc/connectivity"
3031
appsv1 "k8s.io/api/apps/v1"
3132
apimetrics "k8s.io/apiserver/pkg/endpoints/metrics"
33+
"k8s.io/apiserver/pkg/server/healthz"
3234
"k8s.io/client-go/kubernetes/scheme"
3335
kubemetrics "k8s.io/component-base/metrics"
3436
"k8s.io/component-base/metrics/legacyregistry"
@@ -70,20 +72,20 @@ var (
7072
stdErrThreshold string
7173
)
7274

73-
func (a *Adapter) makeProvider(ctx context.Context) (provider.ExternalMetricsProvider, error) {
75+
func (a *Adapter) makeProvider(ctx context.Context) (provider.ExternalMetricsProvider, *metricsservice.GrpcClient, error) {
7476
scheme := scheme.Scheme
7577
if err := appsv1.SchemeBuilder.AddToScheme(scheme); err != nil {
7678
setupLog.Error(err, "failed to add apps/v1 scheme to runtime scheme")
77-
return nil, fmt.Errorf("failed to add apps/v1 scheme to runtime scheme (%s)", err)
79+
return nil, nil, fmt.Errorf("failed to add apps/v1 scheme to runtime scheme (%s)", err)
7880
}
7981
if err := kedav1alpha1.SchemeBuilder.AddToScheme(scheme); err != nil {
8082
setupLog.Error(err, "failed to add keda scheme to runtime scheme")
81-
return nil, fmt.Errorf("failed to add keda scheme to runtime scheme (%s)", err)
83+
return nil, nil, fmt.Errorf("failed to add keda scheme to runtime scheme (%s)", err)
8284
}
8385
namespaces, err := kedautil.GetWatchNamespaces()
8486
if err != nil {
8587
setupLog.Error(err, "failed to get watch namespace")
86-
return nil, fmt.Errorf("failed to get watch namespace (%s)", err)
88+
return nil, nil, fmt.Errorf("failed to get watch namespace (%s)", err)
8789
}
8890

8991
// Get a config to talk to the apiserver
@@ -107,23 +109,23 @@ func (a *Adapter) makeProvider(ctx context.Context) (provider.ExternalMetricsPro
107109
})
108110
if err != nil {
109111
setupLog.Error(err, "failed to setup manager")
110-
return nil, err
112+
return nil, nil, err
111113
}
112114

113115
setupLog.Info("Connecting Metrics Service gRPC client to the server", "address", metricsServiceAddr)
114116
defaultConnectionCfg := ""
115117
grpcClient, err := metricsservice.NewGrpcClient(ctx, metricsServiceAddr, a.SecureServing.ServerCert.CertDirectory, metricsServiceGRPCAuthority, defaultConnectionCfg, clientMetrics, false)
116118
if err != nil {
117119
setupLog.Error(err, "error connecting Metrics Service gRPC client to the server", "address", metricsServiceAddr)
118-
return nil, err
120+
return nil, nil, err
119121
}
120122
go func() {
121123
if err := mgr.Start(ctx); err != nil {
122124
setupLog.Error(err, "controller-runtime encountered an error")
123125
os.Exit(1)
124126
}
125127
}()
126-
return kedaprovider.NewProvider(ctx, setupLog, mgr.GetClient(), *grpcClient), nil
128+
return kedaprovider.NewProvider(ctx, setupLog, mgr.GetClient(), *grpcClient), grpcClient, nil
127129
}
128130

129131
// getMetricHandler returns a http handler that exposes metrics from controller-runtime and apiserver
@@ -305,13 +307,23 @@ func main() {
305307
return
306308
}
307309

308-
kedaProvider, err := cmd.makeProvider(ctx)
310+
kedaProvider, grpcClient, err := cmd.makeProvider(ctx)
309311
if err != nil {
310312
setupLog.Error(err, "making provider")
311313
return
312314
}
313315
cmd.WithExternalMetrics(kedaProvider)
314316

317+
// Register a readiness check that reflects the state of the gRPC connection
318+
// to the KEDA metrics service (keda-operator). Without it the adapter keeps
319+
// reporting Ready even when this connection is down, so the APIService keeps
320+
// routing external-metric requests to a replica that can only fail them.
321+
// See https://github.com/kedacore/keda/issues/8211.
322+
if err = addGrpcConnectionReadyzCheck(cmd, grpcClient); err != nil {
323+
setupLog.Error(err, "unable to register gRPC connection readiness check")
324+
return
325+
}
326+
315327
setupLog.Info(cmd.Message)
316328

317329
RunMetricsServer(ctx)
@@ -320,3 +332,23 @@ func main() {
320332
return
321333
}
322334
}
335+
336+
// addGrpcConnectionReadyzCheck registers a readyz check named
337+
// "metricsservice-grpc-connection" that reports NotReady whenever the gRPC
338+
// connection to the KEDA metrics service is not in the Ready state. A replica
339+
// that cannot establish (or re-establish) the connection is then removed from
340+
// the APIService endpoints so requests are not routed to it.
341+
func addGrpcConnectionReadyzCheck(cmd *Adapter, grpcClient *metricsservice.GrpcClient) error {
342+
server, err := cmd.Server()
343+
if err != nil {
344+
return err
345+
}
346+
return server.GenericAPIServer.AddReadyzChecks(
347+
healthz.NamedCheck("metricsservice-grpc-connection", func(_ *http.Request) error {
348+
if state := grpcClient.GetConnectionState(); state != connectivity.Ready {
349+
return fmt.Errorf("gRPC connection to KEDA metrics service is not ready (state: %s)", state)
350+
}
351+
return nil
352+
}),
353+
)
354+
}

‎pkg/metricsservice/client.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,26 @@ func NewGrpcClient(ctx context.Context, url, certDir, authority, confOptions str
106106
return &grpcClient, nil
107107
}
108108

109+
// GetConnectionState returns the current connectivity state of the underlying
110+
// gRPC connection to the KEDA metrics service.
111+
//
112+
// When the connection is Idle it also triggers a non-blocking reconnection
113+
// attempt (moving it towards Connecting) before returning. This matters for the
114+
// readiness probe: once a non-Ready state removes the replica from the
115+
// APIService endpoints, no RPCs flow through this client, so nothing else would
116+
// ever wake an Idle connection. gRPC parks a connection in Idle after
117+
// GRPC_CLIENT_IDLE_TIMEOUT_MS (30 minutes by default) without RPCs, so without
118+
// this nudge the replica could stay NotReady permanently. Connect() is
119+
// non-blocking and a no-op when the connection is not Idle, so it is safe to
120+
// call from a readiness probe.
121+
func (c *GrpcClient) GetConnectionState() connectivity.State {
122+
state := c.connection.GetState()
123+
if state == connectivity.Idle {
124+
c.connection.Connect()
125+
}
126+
return state
127+
}
128+
109129
func (c *GrpcClient) GetMetrics(ctx context.Context, scaledObjectName, scaledObjectNamespace, metricName string) (*external_metrics.ExternalMetricValueList, error) {
110130
// Fail fast if the gRPC connection has been shut down, rather than
111131
// waiting until the context timeout expires.

‎pkg/metricsservice/client_test.go‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/*
2+
Copyright 2026 The KEDA Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package metricsservice
18+
19+
import (
20+
"testing"
21+
"time"
22+
23+
"github.com/stretchr/testify/require"
24+
"google.golang.org/grpc"
25+
"google.golang.org/grpc/connectivity"
26+
"google.golang.org/grpc/credentials/insecure"
27+
)
28+
29+
// TestGetConnectionState is a regression test for issue #8211: the metrics
30+
// adapter needs a way to observe the gRPC connection state so a readiness
31+
// probe can report NotReady when the connection to the KEDA metrics service
32+
// is down. GetConnectionState must reflect the underlying connection's state
33+
// without blocking.
34+
func TestGetConnectionState(t *testing.T) {
35+
// A freshly created client (grpc.NewClient does not dial until first use)
36+
// starts in the Idle state, which is not Ready. A readiness probe must be
37+
// able to observe this so it does not report a not-yet-connected replica
38+
// as Ready.
39+
conn, err := grpc.NewClient("passthrough:///127.0.0.1:0",
40+
grpc.WithTransportCredentials(insecure.NewCredentials()))
41+
require.NoError(t, err)
42+
defer conn.Close()
43+
44+
c := &GrpcClient{connection: conn}
45+
46+
// A connection that has never dialed is not Ready, so a readiness check
47+
// built on this getter would correctly mark the replica NotReady.
48+
require.NotEqual(t, connectivity.Ready, c.GetConnectionState())
49+
}
50+
51+
// TestGetConnectionStateReconnectsWhenIdle guards against the failure mode
52+
// raised in review on PR #8226: once a non-Ready state removes the replica
53+
// from the APIService endpoints, no RPCs flow through the client, so an Idle
54+
// connection would never be woken and the replica could stay NotReady forever.
55+
// GetConnectionState must therefore trigger a non-blocking reconnect when it
56+
// observes an Idle connection, moving it off Idle so it can recover on its own.
57+
func TestGetConnectionStateReconnectsWhenIdle(t *testing.T) {
58+
conn, err := grpc.NewClient("passthrough:///127.0.0.1:0",
59+
grpc.WithTransportCredentials(insecure.NewCredentials()))
60+
require.NoError(t, err)
61+
defer conn.Close()
62+
63+
c := &GrpcClient{connection: conn}
64+
65+
// A fresh client is parked in Idle until something drives it to connect.
66+
require.Equal(t, connectivity.Idle, conn.GetState())
67+
68+
// Calling the getter on an Idle connection must kick a reconnection
69+
// attempt. The reported state is still the pre-nudge Idle, but the
70+
// underlying connection then leaves Idle without any RPC being issued.
71+
require.Equal(t, connectivity.Idle, c.GetConnectionState())
72+
73+
require.Eventually(t, func() bool {
74+
return conn.GetState() != connectivity.Idle
75+
}, 2*time.Second, 10*time.Millisecond,
76+
"GetConnectionState should trigger a reconnect that moves the connection off Idle")
77+
}

0 commit comments

Comments
 (0)