Skip to content

chore(deps): upgrade k8s.io/client-go to v0.36.2#326

Draft
bernot-dev wants to merge 1 commit into
release-2.53.5-gmpfrom
bump-k8s-deps
Draft

chore(deps): upgrade k8s.io/client-go to v0.36.2#326
bernot-dev wants to merge 1 commit into
release-2.53.5-gmpfrom
bump-k8s-deps

Conversation

@bernot-dev

Copy link
Copy Markdown
Collaborator
  • Upgrade k8s.io/client-go, k8s.io/api, and k8s.io/apimachinery to v0.36.2.
  • Remove obsolete k8s.io/klog/v2 replace directive in go.mod to support klog.NewContext and textlogger required by updated apimachinery.
  • Add klogGokitSink logr.LogSink adapter in cmd/prometheus to bridge klog/v2 with go-kit/log.
  • Update watch error handlers in discovery/kubernetes to use cache.WatchErrorHandlerWithContext.
  • Wrap cache.ListWatch with cache.ToListWatcherWithWatchListSemantics in discovery/kubernetes informers so Reflector correctly detects client capabilities when streaming initial events (WatchListClient feature gate).

- Upgrade k8s.io/client-go, k8s.io/api, and k8s.io/apimachinery to v0.36.2.
- Remove obsolete k8s.io/klog/v2 replace directive in go.mod to support klog.NewContext and textlogger required by updated apimachinery.
- Add klogGokitSink logr.LogSink adapter in cmd/prometheus to bridge klog/v2 with go-kit/log.
- Update watch error handlers in discovery/kubernetes to use cache.WatchErrorHandlerWithContext.
- Wrap cache.ListWatch with cache.ToListWatcherWithWatchListSemantics in discovery/kubernetes informers so Reflector correctly detects client capabilities when streaming initial events (WatchListClient feature gate).
@bernot-dev bernot-dev self-assigned this Jul 1, 2026
@bernot-dev bernot-dev requested a review from bwplotka July 1, 2026 22:05

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request upgrades Go to version 1.26.0, updates various Kubernetes dependencies, and removes the replace directive for klog/v2. It introduces a custom klogGokitSink to bridge klog/v2 with Go-kit logging, and updates Kubernetes discovery informers to utilize watch list semantics and context-aware error handlers. Feedback on these changes highlights that the current WithName implementation in klogGokitSink is a no-op and should be updated to preserve sub-logger names while optimizing slice allocations. Additionally, defensive nil checks for d.client should be added in kubernetes.go to prevent potential nil pointer dereferences.

Comment thread cmd/prometheus/main.go
Comment on lines +1904 to +1930
type klogGokitSink struct {
logger log.Logger
maxLevel int
}

func (s *klogGokitSink) Init(info logr.RuntimeInfo) {}
func (s *klogGokitSink) Enabled(level int) bool { return level <= s.maxLevel }
func (s *klogGokitSink) Info(level int, msg string, keysAndValues ...any) {
if level > s.maxLevel {
return
}
kvs := append([]any{"level", "info", "msg", msg}, keysAndValues...)
_ = s.logger.Log(kvs...)
}
func (s *klogGokitSink) Error(err error, msg string, keysAndValues ...any) {
kvs := append([]any{"level", "error", "msg", msg, "err", err}, keysAndValues...)
_ = s.logger.Log(kvs...)
}
func (s *klogGokitSink) WithValues(keysAndValues ...any) logr.LogSink {
return &klogGokitSink{
logger: log.With(s.logger, keysAndValues...),
maxLevel: s.maxLevel,
}
}
func (s *klogGokitSink) WithName(name string) logr.LogSink {
return s
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation of WithName is a no-op that simply returns s. This completely discards the sub-logger names configured by client-go (e.g., reflector, informer), making it very difficult to identify which component produced a log message.

To preserve this crucial context, we should store the logger name in the klogGokitSink struct, append new names in WithName, copy the name in WithValues, and include it as a "logger" key in the log output when it is not empty. Additionally, we can optimize the slice allocations in Info and Error by pre-allocating the slice with the correct capacity.

type klogGokitSink struct {
	logger   log.Logger
	maxLevel int
	name     string
}

func (s *klogGokitSink) Init(info logr.RuntimeInfo) {}
func (s *klogGokitSink) Enabled(level int) bool { return level <= s.maxLevel }
func (s *klogGokitSink) Info(level int, msg string, keysAndValues ...any) {
	if level > s.maxLevel {
		return
	}
	kvs := make([]any, 0, 4+len(keysAndValues)+2)
	kvs = append(kvs, "level", "info", "msg", msg)
	if s.name != "" {
		kvs = append(kvs, "logger", s.name)
	}
	kvs = append(kvs, keysAndValues...)
	_ = s.logger.Log(kvs...)
}
func (s *klogGokitSink) Error(err error, msg string, keysAndValues ...any) {
	kvs := make([]any, 0, 6+len(keysAndValues)+2)
	kvs = append(kvs, "level", "error", "msg", msg, "err", err)
	if s.name != "" {
		kvs = append(kvs, "logger", s.name)
	}
	kvs = append(kvs, keysAndValues...)
	_ = s.logger.Log(kvs...)
}
func (s *klogGokitSink) WithValues(keysAndValues ...any) logr.LogSink {
	return &klogGokitSink{
		logger:   log.With(s.logger, keysAndValues...),
		maxLevel: s.maxLevel,
		name:     s.name,
	}
}
func (s *klogGokitSink) WithName(name string) logr.LogSink {
	newName := name
	if s.name != "" {
		newName = s.name + "." + name
	}
	return &klogGokitSink{
		logger:   s.logger,
		maxLevel: s.maxLevel,
		name:     newName,
	}
}

Comment on lines +866 to +868
if listWatch, ok := lw.(*cache.ListWatch); ok {
lw = cache.ToListWatcherWithWatchListSemantics(listWatch, d.client)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To prevent potential nil pointer dereferences or panics in unit tests or custom environments where d.client might not be fully initialized, we should defensively check that d.client is not nil before wrapping the ListWatch with cache.ToListWatcherWithWatchListSemantics.

Suggested change
if listWatch, ok := lw.(*cache.ListWatch); ok {
lw = cache.ToListWatcherWithWatchListSemantics(listWatch, d.client)
}
if listWatch, ok := lw.(*cache.ListWatch); ok && d.client != nil {
lw = cache.ToListWatcherWithWatchListSemantics(listWatch, d.client)
}

Comment on lines +879 to +881
if listWatch, ok := lw.(*cache.ListWatch); ok {
lw = cache.ToListWatcherWithWatchListSemantics(listWatch, d.client)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To prevent potential nil pointer dereferences or panics in unit tests or custom environments where d.client might not be fully initialized, we should defensively check that d.client is not nil before wrapping the ListWatch with cache.ToListWatcherWithWatchListSemantics.

Suggested change
if listWatch, ok := lw.(*cache.ListWatch); ok {
lw = cache.ToListWatcherWithWatchListSemantics(listWatch, d.client)
}
if listWatch, ok := lw.(*cache.ListWatch); ok && d.client != nil {
lw = cache.ToListWatcherWithWatchListSemantics(listWatch, d.client)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant