Skip to content

Commit

Permalink
chore: decouple diagnostics server from collector and handlers
Browse files Browse the repository at this point in the history
  • Loading branch information
czeslavo committed Feb 5, 2025
1 parent e61ca89 commit 7af6dbd
Show file tree
Hide file tree
Showing 18 changed files with 924 additions and 460 deletions.
23 changes: 18 additions & 5 deletions internal/dataplane/kong_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ type KongClient struct {

// diagnostic is the client and configuration for reporting diagnostic
// information during data-plane update runtime.
diagnostic diagnostics.ClientDiagnostic
diagnostic diagnostics.Client

// metricsRecorder is the client for shipping metrics information
// updates to the prometheus exporter.
Expand Down Expand Up @@ -189,12 +189,21 @@ type KongClient struct {
konnectKongStateUpdater KonnectKongStateUpdater
}

// KongClientOption is a functional option for configuring a KongClient.
type KongClientOption func(*KongClient)

// WithDiagnosticClient sets the diagnostic client for the KongClient.
func WithDiagnosticClient(diagnostic diagnostics.Client) func(*KongClient) {
return func(c *KongClient) {
c.diagnostic = diagnostic
}
}

// NewKongClient provides a new KongClient object after connecting to the
// data-plane API and verifying integrity.
func NewKongClient(
logger logr.Logger,
timeout time.Duration,
diagnostic diagnostics.ClientDiagnostic,
kongConfig sendconfig.Config,
eventRecorder record.EventRecorder,
dbMode dpconf.DBMode,
Expand All @@ -206,11 +215,11 @@ func NewKongClient(
cacheStores *store.CacheStores,
fallbackConfigGenerator FallbackConfigGenerator,
metricsRecorder metrics.Recorder,
opts ...KongClientOption,
) (*KongClient, error) {
c := &KongClient{
logger: logger,
requestTimeout: timeout,
diagnostic: diagnostic,
metricsRecorder: metricsRecorder,
cache: cacheStores,
kongConfig: kongConfig,
Expand All @@ -226,6 +235,10 @@ func NewKongClient(
}
c.initializeControllerPodReference()

for _, opt := range opts {
opt(c)
}

return c, nil
}

Expand Down Expand Up @@ -849,13 +862,13 @@ type sendDiagnosticFn func(meta diagnostics.DumpMeta, raw []byte)
func prepareSendDiagnosticFn(
ctx context.Context,
logger logr.Logger,
diagnosticConfig diagnostics.ClientDiagnostic,
diagnosticConfig diagnostics.Client,
targetState *kongstate.KongState,
targetContent *file.Content,
deckGenParams deckgen.GenerateDeckContentParams,
isFallback bool,
) sendDiagnosticFn {
if diagnosticConfig == (diagnostics.ClientDiagnostic{}) {
if diagnosticConfig == (diagnostics.Client{}) {
// noop, diagnostics won't be sent
return func(diagnostics.DumpMeta, []byte) {}
}
Expand Down
2 changes: 0 additions & 2 deletions internal/dataplane/kong_client_golden_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/fallback"
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/sendconfig"
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator"
"github.com/kong/kubernetes-ingress-controller/v3/internal/diagnostics"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/consts"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/featuregates"
"github.com/kong/kubernetes-ingress-controller/v3/internal/store"
Expand Down Expand Up @@ -301,7 +300,6 @@ func runKongClientGoldenTest(t *testing.T, tc kongClientGoldenTestCase) {
kongClient, err := NewKongClient(
logger,
timeout,
diagnostics.ClientDiagnostic{},
cfg,
mocks.NewEventRecorder(),
dpconf.DBModeOff, // Test will run in DB-less mode only for now. In the future, we may want to test DB mode as well.
Expand Down
24 changes: 10 additions & 14 deletions internal/dataplane/kong_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,6 @@ func setupTestKongClient(
) *KongClient {
logger := zapr.NewLogger(zap.NewNop())
timeout := time.Second
diagnostic := diagnostics.ClientDiagnostic{}
config := sendconfig.Config{
SanitizeKonnectConfigDumps: true,
}
Expand All @@ -802,7 +801,6 @@ func setupTestKongClient(
kongClient, err := NewKongClient(
logger,
timeout,
diagnostic,
config,
eventRecorder,
dpconf.DBModeOff,
Expand Down Expand Up @@ -1021,9 +1019,6 @@ func TestKongClient_FallbackConfiguration_SuccessfulRecovery(t *testing.T) {
kongClient, err := NewKongClient(
zapr.NewLogger(zap.NewNop()),
time.Second,
diagnostics.ClientDiagnostic{
Configs: diagnosticsCh,
},
sendconfig.Config{
FallbackConfiguration: true,
UseLastValidConfigForFallback: tc.enableLastValidConfigFallback,
Expand All @@ -1038,6 +1033,9 @@ func TestKongClient_FallbackConfiguration_SuccessfulRecovery(t *testing.T) {
&originalCache,
fallbackConfigGenerator,
mocks.MetricsRecorder{},
WithDiagnosticClient(diagnostics.Client{
Configs: diagnosticsCh,
}),
)
require.NoError(t, err)

Expand Down Expand Up @@ -1156,9 +1154,6 @@ func TestKongClient_FallbackConfiguration_SkipsUpdateWhenInSync(t *testing.T) {
kongClient, err := NewKongClient(
zapr.NewLogger(zap.NewNop()),
time.Second,
diagnostics.ClientDiagnostic{
Configs: diagnosticsCh,
},
sendconfig.Config{
FallbackConfiguration: true,
},
Expand All @@ -1172,6 +1167,9 @@ func TestKongClient_FallbackConfiguration_SkipsUpdateWhenInSync(t *testing.T) {
&originalCache,
fallbackConfigGenerator,
mocks.MetricsRecorder{},
WithDiagnosticClient(diagnostics.Client{
Configs: diagnosticsCh,
}),
)
require.NoError(t, err)

Expand Down Expand Up @@ -1301,9 +1299,6 @@ func TestKongClient_FallbackConfiguration_FailedRecovery(t *testing.T) {
kongClient, err := NewKongClient(
zapr.NewLogger(zap.NewNop()),
time.Second,
diagnostics.ClientDiagnostic{
Configs: diagnosticsCh,
},
sendconfig.Config{
FallbackConfiguration: true,
},
Expand All @@ -1317,6 +1312,9 @@ func TestKongClient_FallbackConfiguration_FailedRecovery(t *testing.T) {
&originalCache,
fallbackConfigGenerator,
mocks.MetricsRecorder{},
WithDiagnosticClient(diagnostics.Client{
Configs: diagnosticsCh,
}),
)
require.NoError(t, err)

Expand Down Expand Up @@ -1410,7 +1408,6 @@ func TestKongClient_LastValidCacheSnapshot(t *testing.T) {
kongClient, err := NewKongClient(
zapr.NewLogger(zap.NewNop()),
time.Second,
diagnostics.ClientDiagnostic{},
sendconfig.Config{
FallbackConfiguration: tc.fallbackConfigurationFeatureEnabled,
UseLastValidConfigForFallback: tc.useLastValidConfigForFallbackEnabled,
Expand Down Expand Up @@ -1498,7 +1495,7 @@ func TestKongClient_ConfigDumpSanitization(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
diagnosticsCh := make(chan diagnostics.ConfigDump, 1) // make it buffered to avoid blocking
kongClient.diagnostic = diagnostics.ClientDiagnostic{
kongClient.diagnostic = diagnostics.Client{
Configs: diagnosticsCh,
DumpsIncludeSensitive: tc.dumpsIncludeSensitive,
}
Expand Down Expand Up @@ -1633,7 +1630,6 @@ func TestKongClient_RecoveringFromGatewaySyncError(t *testing.T) {
kongClient, err := NewKongClient(
zapr.NewLogger(zap.NewNop()),
time.Second,
diagnostics.ClientDiagnostic{},
sendconfig.Config{
FallbackConfiguration: true,
},
Expand Down
6 changes: 3 additions & 3 deletions internal/dataplane/sendconfig/dbmode.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type UpdateStrategyDBMode struct {
dumpConfig dump.Config
version semver.Version
concurrency int
diagnostic *diagnostics.ClientDiagnostic
diagnostic *diagnostics.Client
isKonnect bool
logger logr.Logger
resourceErrors []ResourceError
Expand All @@ -42,7 +42,7 @@ type UpdateStrategyDBMode struct {
type UpdateStrategyDBModeOpt func(*UpdateStrategyDBMode)

// WithDiagnostic sets the diagnostic server to send diffs to.
func WithDiagnostic(diagnostic *diagnostics.ClientDiagnostic) UpdateStrategyDBModeOpt {
func WithDiagnostic(diagnostic *diagnostics.Client) UpdateStrategyDBModeOpt {
return func(s *UpdateStrategyDBMode) {
s.diagnostic = diagnostic
}
Expand Down Expand Up @@ -137,7 +137,7 @@ func (s *UpdateStrategyDBMode) Update(ctx context.Context, targetContent Content
func (s *UpdateStrategyDBMode) HandleEvents(
ctx context.Context,
events chan diff.EntityAction,
diagnostic *diagnostics.ClientDiagnostic,
diagnostic *diagnostics.Client,
hash string,
) {
s.resourceErrorLock.Lock()
Expand Down
4 changes: 2 additions & 2 deletions internal/dataplane/sendconfig/sendconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
// -----------------------------------------------------------------------------

type UpdateStrategyResolver interface {
ResolveUpdateStrategy(client UpdateClient, diagnostic *diagnostics.ClientDiagnostic) UpdateStrategy
ResolveUpdateStrategy(client UpdateClient, diagnostic *diagnostics.Client) UpdateStrategy
}

type AdminAPIClient interface {
Expand All @@ -49,7 +49,7 @@ func PerformUpdate(
promMetrics metrics.Recorder,
updateStrategyResolver UpdateStrategyResolver,
configChangeDetector ConfigurationChangeDetector,
diagnostic *diagnostics.ClientDiagnostic,
diagnostic *diagnostics.Client,
isFallback bool,
) ([]byte, error) {
oldSHA := client.LastConfigSHA()
Expand Down
4 changes: 2 additions & 2 deletions internal/dataplane/sendconfig/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func NewDefaultUpdateStrategyResolver(config Config, logger logr.Logger) Default
// with the backoff strategy it provides.
func (r DefaultUpdateStrategyResolver) ResolveUpdateStrategy(
client UpdateClient,
diagnostic *diagnostics.ClientDiagnostic,
diagnostic *diagnostics.Client,
) UpdateStrategy {
updateStrategy := r.resolveUpdateStrategy(client, diagnostic)

Expand All @@ -94,7 +94,7 @@ func (r DefaultUpdateStrategyResolver) ResolveUpdateStrategy(

func (r DefaultUpdateStrategyResolver) resolveUpdateStrategy(
client UpdateClient,
diagnostic *diagnostics.ClientDiagnostic,
diagnostic *diagnostics.Client,
) UpdateStrategy {
adminAPIClient := client.AdminAPIClient()

Expand Down
Loading

0 comments on commit 7af6dbd

Please sign in to comment.