Skip to content

[chore] Introduce client registry and provider to centralise Valkey client instantiation #266

Description

@jdheyburn

Problem

Valkey client creation is scattered across multiple controllers and utility files. Each caller constructs its own connection with its own TLS config, credential lookup, and error handling. This makes it hard to add cross-cutting concerns (retry logic, connection pooling, observability) consistently, and leads to subtle divergences between how different parts of the code connect to pods.

This was noted as a future improvement in PR #228:

"I would like to propose a client registry that handles this in the future."

Proposed solution

Introduce an internal ClientProvider interface and a ClientRegistry implementation:

type ClientProvider interface {
    // ForNode returns a connected client for the given ValkeyNode.
    ForNode(ctx context.Context, node *valkeyiov1alpha1.ValkeyNode) (Client, error)
    // ForPod returns a connected client for a pod IP and port.
    ForPod(ctx context.Context, podIP string, port int, opts ...ClientOption) (Client, error)
}

The registry handles TLS config resolution, credential lookup, and connection reuse. All controllers go through this interface instead of instantiating clients directly. Tests inject a fake via the interface.

Acceptance criteria

  • ClientProvider interface defined in internal/client/
  • All direct vclient.NewClient calls in controllers replaced with ClientProvider.ForNode or ForPod
  • Retry/backoff policy centralised in the registry
  • FakeClientProvider available for unit tests
  • No behaviour change in existing functionality

References

Metadata

Metadata

Assignees

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions