Skip to content

Commit 3e54797

Browse files
elevranelevran
authored
Remove use of klog package from sidecar (#471)
* replace ktesting with explicit calls Signed-off-by: Etai Lev Ran <elevran@gmail.com> * replace klog with controller-runtime/log Signed-off-by: Etai Lev Ran <elevran@gmail.com> * remove klog Signed-off-by: Etai Lev Ran <elevran@gmail.com> * lint fixes (revive var-naming was not introduced in this PR) Signed-off-by: Etai Lev Ran <elevran@gmail.com> --------- Signed-off-by: Etai Lev Ran <elevran@gmail.com>
1 parent ea0a0cb commit 3e54797

File tree

9 files changed

+37
-20
lines changed

9 files changed

+37
-20
lines changed

‎cmd/pd-sidecar/main.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,9 @@ import (
2323
"strconv"
2424
"strings"
2525

26-
"k8s.io/klog/v2"
2726
ctrl "sigs.k8s.io/controller-runtime"
27+
"sigs.k8s.io/controller-runtime/pkg/log"
28+
"sigs.k8s.io/controller-runtime/pkg/log/zap"
2829

2930
"github.com/llm-d/llm-d-inference-scheduler/pkg/sidecar/proxy"
3031
"github.com/llm-d/llm-d-inference-scheduler/pkg/sidecar/version"
@@ -58,14 +59,15 @@ func main() {
5859
inferencePoolName := flag.String("inference-pool-name", os.Getenv("INFERENCE_POOL_NAME"), "the specific InferencePool name to watch (defaults to INFERENCE_POOL_NAME env var)")
5960
enablePrefillerSampling := flag.Bool("enable-prefiller-sampling", func() bool { b, _ := strconv.ParseBool(os.Getenv("ENABLE_PREFILLER_SAMPLING")); return b }(), "if true, the target prefill instance will be selected randomly from among the provided prefill host values")
6061

61-
klog.InitFlags(nil)
62+
opts := zap.Options{}
63+
opts.BindFlags(flag.CommandLine) // optional to allow zap logging control via CLI
6264
flag.Parse()
6365

64-
// make sure to flush logs before exiting
65-
defer klog.Flush()
66+
logger := zap.New(zap.UseFlagOptions(&opts))
67+
log.SetLogger(logger)
6668

6769
ctx := ctrl.SetupSignalHandler()
68-
logger := klog.FromContext(ctx)
70+
log.IntoContext(ctx, logger)
6971

7072
logger.Info("Proxy starting", "Built on", version.BuildRef, "From Git SHA", version.CommitSHA)
7173

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ require (
2323
k8s.io/apimachinery v0.34.2
2424
k8s.io/client-go v0.34.2
2525
k8s.io/component-base v0.34.2
26-
k8s.io/klog/v2 v2.130.1
2726
k8s.io/utils v0.0.0-20250820121507-0af2bda4dd1d
2827
sigs.k8s.io/controller-runtime v0.22.4
2928
sigs.k8s.io/gateway-api v1.4.0
@@ -126,6 +125,7 @@ require (
126125
gopkg.in/inf.v0 v0.9.1 // indirect
127126
gopkg.in/yaml.v3 v3.0.1 // indirect
128127
k8s.io/apiserver v0.34.2 // indirect
128+
k8s.io/klog/v2 v2.130.1 // indirect
129129
k8s.io/kube-openapi v0.0.0-20250814151709-d7b6acb124c3 // indirect
130130
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.2 // indirect
131131
sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 // indirect

‎pkg/sidecar/proxy/allowlist.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ import (
3333
"k8s.io/client-go/dynamic"
3434
"k8s.io/client-go/tools/cache"
3535
"k8s.io/client-go/tools/clientcmd"
36-
"k8s.io/klog/v2"
3736
"k8s.io/utils/set"
37+
"sigs.k8s.io/controller-runtime/pkg/log"
3838
)
3939

4040
const (
@@ -105,7 +105,7 @@ func (av *AllowlistValidator) Start(ctx context.Context) error {
105105
return nil
106106
}
107107

108-
av.logger = klog.FromContext(ctx).WithName("allowlist-validator")
108+
av.logger = log.FromContext(ctx).WithName("allowlist-validator")
109109
av.logger.Info("starting SSRF protection allowlist validator", "namespace", av.namespace, "poolName", av.poolName)
110110

111111
gvr := schema.GroupVersionResource{

‎pkg/sidecar/proxy/connector_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030
"github.com/llm-d/llm-d-inference-scheduler/test/sidecar/mock"
3131
. "github.com/onsi/ginkgo/v2" // nolint:revive
3232
. "github.com/onsi/gomega" // nolint:revive
33-
"k8s.io/klog/v2/ktesting"
3433
)
3534

3635
type sidecarTestInfo struct {
@@ -180,7 +179,7 @@ var _ = Describe("Common Connector tests", func() {
180179
func sidecarConnectionTestSetup(connector string) *sidecarTestInfo {
181180
testInfo := sidecarTestInfo{}
182181

183-
_, testInfo.ctx = ktesting.NewTestContext(GinkgoT())
182+
testInfo.ctx = newTestContext()
184183
testInfo.ctx, testInfo.cancelFn = context.WithCancel(testInfo.ctx)
185184
testInfo.stoppedCh = make(chan struct{})
186185

‎pkg/sidecar/proxy/data_parallel.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111

1212
"github.com/llm-d/llm-d-inference-scheduler/pkg/common"
1313
"golang.org/x/sync/errgroup"
14-
"k8s.io/klog/v2"
14+
"sigs.k8s.io/controller-runtime/pkg/log"
1515
)
1616

1717
// dataParallelHandler checks if Data Parallel handling is needed.
@@ -71,7 +71,7 @@ func (s *Server) startDataParallel(ctx context.Context, cert *tls.Certificate, g
7171
}
7272

7373
clone := s.Clone()
74-
clone.logger = klog.FromContext(ctx).WithName("proxy server on port " + rankPort)
74+
clone.logger = log.FromContext(ctx).WithName("proxy server on port " + rankPort)
7575
clone.port = rankPort
7676
clone.decoderURL = decoderURL
7777
clone.forwardDataParallel = false

‎pkg/sidecar/proxy/data_parallel_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
. "github.com/onsi/ginkgo/v2" // nolint:revive
1111
. "github.com/onsi/gomega" // nolint:revive
1212
"golang.org/x/sync/errgroup"
13-
"k8s.io/klog/v2/ktesting"
1413

1514
"github.com/llm-d/llm-d-inference-scheduler/pkg/common"
1615
sidecarmock "github.com/llm-d/llm-d-inference-scheduler/test/sidecar/mock"
@@ -24,7 +23,7 @@ const (
2423
var _ = Describe("Data Parallel support", func() {
2524
When("configured with --data-parallel-size > 1", func() {
2625
It("should create an extra proxy", func() {
27-
_, ctx := ktesting.NewTestContext(GinkgoT())
26+
ctx := newTestContext()
2827
ctx, cancel := context.WithCancel(ctx)
2928
grp, ctx := errgroup.WithContext(ctx)
3029

‎pkg/sidecar/proxy/proxy.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import (
2929
"github.com/go-logr/logr"
3030
lru "github.com/hashicorp/golang-lru/v2"
3131
"golang.org/x/sync/errgroup"
32-
"k8s.io/klog/v2"
32+
"sigs.k8s.io/controller-runtime/pkg/log"
3333
)
3434

3535
const (
@@ -141,7 +141,7 @@ func NewProxy(port string, decodeURL *url.URL, config Config) *Server {
141141

142142
// Start the HTTP reverse proxy.
143143
func (s *Server) Start(ctx context.Context, cert *tls.Certificate, allowlistValidator *AllowlistValidator) error {
144-
s.logger = klog.FromContext(ctx).WithName("proxy server on port " + s.port)
144+
s.logger = log.FromContext(ctx).WithName("proxy server on port " + s.port)
145145

146146
s.allowlistValidator = allowlistValidator
147147

‎pkg/sidecar/proxy/proxy_test.go‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,33 @@ import (
2626
"strings"
2727
"time"
2828

29+
"sigs.k8s.io/controller-runtime/pkg/log"
30+
"sigs.k8s.io/controller-runtime/pkg/log/zap"
31+
2932
"github.com/llm-d/llm-d-inference-scheduler/pkg/common"
3033
"github.com/llm-d/llm-d-inference-scheduler/test/sidecar/mock"
3134
. "github.com/onsi/ginkgo/v2" // nolint:revive
3235
. "github.com/onsi/gomega" // nolint:revive
33-
"k8s.io/klog/v2/ktesting"
3436
)
3537

38+
func newTestContext() context.Context {
39+
logger := zap.New(
40+
zap.WriteTo(GinkgoWriter),
41+
zap.UseDevMode(true),
42+
)
43+
log.SetLogger(logger)
44+
ctx := context.Background()
45+
log.IntoContext(ctx, logger) // not strictly needed since we called SetLogger to set default
46+
return ctx
47+
}
48+
3649
var _ = Describe("Reverse Proxy", func() {
3750
When("x-prefiller-url is not present", func() {
3851
DescribeTable("should forward requests to decode server",
3952

4053
func(path string, secureProxy bool) {
41-
_, ctx := ktesting.NewTestContext(GinkgoT())
4254

55+
ctx := newTestContext()
4356
var cert *tls.Certificate
4457
if secureProxy {
4558
tempCert, err := CreateSelfSignedTLSCertificate()
@@ -159,7 +172,7 @@ var _ = Describe("Reverse Proxy", func() {
159172
})
160173

161174
It("should successfully send request to 1. prefill 2. decode with the right fields (backward compatible behavior)", func() {
162-
_, ctx := ktesting.NewTestContext(GinkgoT())
175+
ctx := newTestContext()
163176
ctx, cancelFn := context.WithCancel(ctx)
164177
stoppedCh := make(chan struct{})
165178

@@ -233,7 +246,7 @@ var _ = Describe("Reverse Proxy", func() {
233246
})
234247

235248
It("should successfully send request to 1. prefill 2. decode with the right fields", func() {
236-
_, ctx := ktesting.NewTestContext(GinkgoT())
249+
ctx := newTestContext()
237250
ctx, cancelFn := context.WithCancel(ctx)
238251
stoppedCh := make(chan struct{})
239252

‎test/utils/network.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
// Package utils contains utilities for testing
2+
//
3+
//revive:disable:var-naming
24
package utils
35

6+
//revive:enable:var-naming
7+
48
import (
59
"net"
610
)

0 commit comments

Comments
 (0)