Skip to content

Commit 9682cce

Browse files
committed
inject PodMetricsClient via SetUp
1 parent 22808d1 commit 9682cce

5 files changed

Lines changed: 104 additions & 67 deletions

File tree

cmd/epp/runner/runner.go

Lines changed: 23 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package runner
1818

1919
import (
2020
"context"
21-
"crypto/tls"
2221
"errors"
2322
"flag"
2423
"fmt"
@@ -121,8 +120,7 @@ type Runner struct {
121120
customCollectors []prometheus.Collector
122121

123122
// Test overrides
124-
testPodMetricsClient *backendmetrics.FakePodMetricsClient
125-
skipNameValidation bool
123+
skipNameValidation bool
126124
}
127125

128126
// WithExecutableName sets the name of the executable containing the runner.
@@ -188,7 +186,22 @@ func (r *Runner) Run(ctx context.Context) error {
188186
return err
189187
}
190188

191-
mgr, _, _, err := r.Setup(ctx, cfg, opts)
189+
pmc, err := backendmetrics.NewPodMetricsClientImpl(setupLog, backendmetrics.Config{
190+
ModelServerMetricsScheme: opts.ModelServerMetricsScheme,
191+
ModelServerMetricsHTTPSInsecure: opts.ModelServerMetricsHTTPSInsecure,
192+
ModelServerMetricsPath: opts.ModelServerMetricsPath,
193+
194+
TotalQueuedRequestsMetric: opts.TotalQueuedRequestsMetric,
195+
TotalRunningRequestsMetric: opts.TotalRunningRequestsMetric,
196+
KVCacheUsagePercentageMetric: opts.KVCacheUsagePercentageMetric,
197+
LoRAInfoMetric: opts.LoRAInfoMetric,
198+
CacheInfoMetric: opts.CacheInfoMetric,
199+
})
200+
if err != nil {
201+
return err
202+
}
203+
204+
mgr, _, _, err := r.Setup(ctx, cfg, opts, pmc)
192205
if err != nil {
193206
return err
194207
}
@@ -209,20 +222,15 @@ func (r *Runner) Run(ctx context.Context) error {
209222
// without starting it, allowing for flexible in integration test.
210223
//
211224
// The returned Datastore and ExtProcServerRunner is **only** meant to use in the integration test.
212-
func (r *Runner) Setup(ctx context.Context, cfg *rest.Config, opts *runserver.Options) (ctrl.Manager, datastore.Datastore, *runserver.ExtProcServerRunner, error) {
225+
func (r *Runner) Setup(ctx context.Context, cfg *rest.Config, opts *runserver.Options, pmc backendmetrics.PodMetricsClient) (ctrl.Manager, datastore.Datastore, *runserver.ExtProcServerRunner, error) {
213226
rawConfig, err := r.parseConfigurationPhaseOne(ctx, opts)
214227
if err != nil {
215228
setupLog.Error(err, "Failed to parse configuration")
216229
return nil, nil, nil, err
217230
}
218231

219232
// --- Setup Datastore ---
220-
epf, err := r.setupMetricsCollection(r.featureGates[datalayer.ExperimentalDatalayerFeatureGate], opts)
221-
222-
if r.testPodMetricsClient != nil {
223-
// Test only, override epf with FakePodMetricsClient.
224-
epf = backendmetrics.NewPodMetricsFactory(r.testPodMetricsClient, 10*time.Millisecond)
225-
}
233+
epf, err := r.setupMetricsCollection(r.featureGates[datalayer.ExperimentalDatalayerFeatureGate], opts, pmc)
226234

227235
if err != nil {
228236
return nil, nil, nil, err
@@ -368,10 +376,6 @@ func (r *Runner) Setup(ctx context.Context, cfg *rest.Config, opts *runserver.Op
368376
UseExperimentalDatalayerV2: r.featureGates[datalayer.ExperimentalDatalayerFeatureGate], // pluggable data layer feature flag
369377
}
370378

371-
if r.testPodMetricsClient != nil {
372-
serverRunner.TestPodMetricsClient = r.testPodMetricsClient
373-
}
374-
375379
if err := serverRunner.SetupWithManager(mgr); err != nil {
376380
setupLog.Error(err, "Failed to setup EPP controllers")
377381
return nil, nil, nil, err
@@ -625,46 +629,15 @@ func (r *Runner) setupDataLayer(enableNewMetrics bool, cfg *datalayer.Config,
625629
return nil
626630
}
627631

628-
func (r *Runner) setupMetricsCollection(enableNewMetrics bool, opts *runserver.Options) (datalayer.EndpointFactory, error) {
632+
func (r *Runner) setupMetricsCollection(enableNewMetrics bool, opts *runserver.Options, pmc backendmetrics.PodMetricsClient) (datalayer.EndpointFactory, error) {
629633
if enableNewMetrics {
630634
return datalayer.NewEndpointFactory(nil, opts.RefreshMetricsInterval), nil
631635
}
632-
return setupMetricsV1(opts)
636+
return setupMetricsV1(opts, pmc)
633637
}
634638

635-
func setupMetricsV1(opts *runserver.Options) (datalayer.EndpointFactory, error) {
636-
mapping, err := backendmetrics.NewMetricMapping(
637-
opts.TotalQueuedRequestsMetric,
638-
opts.TotalRunningRequestsMetric,
639-
opts.KVCacheUsagePercentageMetric,
640-
opts.LoRAInfoMetric,
641-
opts.CacheInfoMetric,
642-
)
643-
if err != nil {
644-
setupLog.Error(err, "Failed to create metric mapping from flags.")
645-
return nil, err
646-
}
647-
verifyMetricMapping(*mapping)
648-
649-
var metricsHttpClient *http.Client
650-
if opts.ModelServerMetricsScheme == "https" {
651-
metricsHttpClient = &http.Client{
652-
Transport: &http.Transport{
653-
TLSClientConfig: &tls.Config{
654-
InsecureSkipVerify: opts.ModelServerMetricsHTTPSInsecure,
655-
},
656-
},
657-
}
658-
} else {
659-
metricsHttpClient = http.DefaultClient
660-
}
661-
662-
pmf := backendmetrics.NewPodMetricsFactory(&backendmetrics.PodMetricsClientImpl{
663-
MetricMapping: mapping,
664-
ModelServerMetricsPath: opts.ModelServerMetricsPath,
665-
ModelServerMetricsScheme: opts.ModelServerMetricsScheme,
666-
Client: metricsHttpClient,
667-
},
639+
func setupMetricsV1(opts *runserver.Options, pmc backendmetrics.PodMetricsClient) (datalayer.EndpointFactory, error) {
640+
pmf := backendmetrics.NewPodMetricsFactory(pmc,
668641
opts.RefreshMetricsInterval)
669642
return pmf, nil
670643
}

cmd/epp/runner/test_runner.go

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,22 @@ limitations under the License.
1717
package runner
1818

1919
import (
20-
backendmetrics "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/backend/metrics"
20+
"github.com/prometheus/client_golang/prometheus"
21+
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/requestcontrol"
2122
)
2223

23-
// WithTestPodMetricsClient sets the fake pod metrics client for integration tests.
24-
func (r *Runner) WithTestPodMetricsClient(client *backendmetrics.FakePodMetricsClient) *Runner {
25-
r.testPodMetricsClient = client
26-
return r
24+
// NewTestRunner creates a runner dedicated for integration test.
25+
func NewTestRunner() *Runner {
26+
runner := &Runner{
27+
eppExecutableName: "GIEIntegrationTest",
28+
requestControlConfig: requestcontrol.NewConfig(), // default requestcontrol config has empty plugin list
29+
customCollectors: []prometheus.Collector{},
30+
}
31+
runner.withSkipNameValidation(true)
32+
return runner
2733
}
2834

29-
// WithSkipNameValidation sets the flag to skip name validation for integration tests.
30-
func (r *Runner) WithSkipNameValidation(skip bool) *Runner {
35+
func (r *Runner) withSkipNameValidation(skip bool) *Runner {
3136
r.skipNameValidation = skip
3237
return r
3338
}

pkg/epp/backend/metrics/metrics.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,13 @@ package metrics
1818

1919
import (
2020
"context"
21+
"crypto/tls"
2122
"fmt"
2223
"net/http"
2324
"strconv"
2425
"strings"
2526

27+
"github.com/go-logr/logr"
2628
dto "github.com/prometheus/client_model/go"
2729
"github.com/prometheus/common/expfmt"
2830
"github.com/prometheus/common/model"
@@ -49,6 +51,66 @@ type PodMetricsClientImpl struct {
4951
Client *http.Client
5052
}
5153

54+
type Config struct {
55+
ModelServerMetricsScheme string
56+
ModelServerMetricsHTTPSInsecure bool
57+
ModelServerMetricsPath string
58+
59+
TotalQueuedRequestsMetric string
60+
TotalRunningRequestsMetric string
61+
KVCacheUsagePercentageMetric string
62+
LoRAInfoMetric string
63+
CacheInfoMetric string
64+
}
65+
66+
func NewPodMetricsClientImpl(logger logr.Logger, config Config) (PodMetricsClient, error) {
67+
mapping, err := NewMetricMapping(
68+
config.TotalQueuedRequestsMetric,
69+
config.TotalRunningRequestsMetric,
70+
config.KVCacheUsagePercentageMetric,
71+
config.LoRAInfoMetric,
72+
config.CacheInfoMetric,
73+
)
74+
if err != nil {
75+
return nil, err
76+
}
77+
verifyMetricMapping(logger, *mapping)
78+
79+
var metricsHttpClient *http.Client
80+
if config.ModelServerMetricsScheme == "https" {
81+
metricsHttpClient = &http.Client{
82+
Transport: &http.Transport{
83+
TLSClientConfig: &tls.Config{
84+
InsecureSkipVerify: config.ModelServerMetricsHTTPSInsecure,
85+
},
86+
},
87+
}
88+
} else {
89+
metricsHttpClient = http.DefaultClient
90+
}
91+
return &PodMetricsClientImpl{
92+
MetricMapping: mapping,
93+
ModelServerMetricsPath: config.ModelServerMetricsPath,
94+
ModelServerMetricsScheme: config.ModelServerMetricsScheme,
95+
Client: metricsHttpClient,
96+
}, nil
97+
}
98+
99+
func verifyMetricMapping(logger logr.Logger, mapping MetricMapping) {
100+
if mapping.TotalQueuedRequests == nil {
101+
logger.Info("Not scraping metric: TotalQueuedRequests")
102+
}
103+
if mapping.KVCacheUtilization == nil {
104+
logger.Info("Not scraping metric: KVCacheUtilization")
105+
}
106+
if mapping.LoraRequestInfo == nil {
107+
logger.Info("Not scraping metric: LoraRequestInfo")
108+
}
109+
if mapping.CacheConfigInfo == nil {
110+
logger.Info("Not scraping metric: CacheConfigInfo")
111+
}
112+
}
113+
52114
// FetchMetrics fetches metrics from a given pod, clones the existing metrics object and returns an updated one.
53115
func (p *PodMetricsClientImpl) FetchMetrics(ctx context.Context, metadata *fwkdl.EndpointMetadata, existing *MetricsState) (*MetricsState, error) {
54116
url := p.getMetricEndpoint(metadata)

pkg/epp/server/runserver.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,6 @@ type ExtProcServerRunner struct {
6060
Director *requestcontrol.Director
6161
SaturationDetector *utilizationdetector.Detector
6262
UseExperimentalDatalayerV2 bool // Pluggable data layer feature flag
63-
64-
// This should only be used in tests. We won't need this once we do not inject metrics in the tests.
65-
// TODO:(https://github.com/kubernetes-sigs/gateway-api-inference-extension/issues/432) Cleanup
66-
TestPodMetricsClient *backendmetrics.FakePodMetricsClient
6763
}
6864

6965
// NewDefaultExtProcServerRunner creates a runner with default values.

test/integration/epp/harness.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ type TestHarness struct {
9494

9595
// Internal handles for cleanup
9696
grpcConn *grpc.ClientConn
97+
98+
fakePmc *backendmetrics.FakePodMetricsClient
9799
}
98100

99101
// NewTestHarness boots up a fully isolated test environment.
@@ -106,12 +108,10 @@ func NewTestHarness(t *testing.T, ctx context.Context, eppOptions *server.Option
106108
for _, opt := range opts {
107109
opt(config)
108110
}
109-
eppRunner := eppRunner.NewRunner()
110-
111-
testPodMetricsClient := &backendmetrics.FakePodMetricsClient{}
112-
eppRunner.WithTestPodMetricsClient(testPodMetricsClient).WithSkipNameValidation(true)
113111

114-
mgr, dataStore, runner, err := eppRunner.Setup(ctx, testEnv.Config, eppOptions)
112+
fakePmc := &backendmetrics.FakePodMetricsClient{}
113+
eppRunner := eppRunner.NewTestRunner()
114+
mgr, dataStore, runner, err := eppRunner.Setup(ctx, testEnv.Config, eppOptions, fakePmc)
115115
require.NoError(t, err, "failed to create manager")
116116
mgrCtx, mgrCancel := context.WithCancel(ctx)
117117

@@ -141,6 +141,7 @@ func NewTestHarness(t *testing.T, ctx context.Context, eppOptions *server.Option
141141
Client: client,
142142
Datastore: dataStore,
143143
grpcConn: conn,
144+
fakePmc: fakePmc,
144145
}
145146

146147
t.Cleanup(func() {
@@ -190,7 +191,7 @@ func (h *TestHarness) WithPods(pods []podState) *TestHarness {
190191
WaitingModels: make(map[string]int),
191192
}
192193
}
193-
h.ServerRunner.TestPodMetricsClient.SetRes(metricsMap)
194+
h.fakePmc.SetRes(metricsMap)
194195

195196
// Create K8s Objects.
196197
for _, p := range pods {

0 commit comments

Comments
 (0)