Skip to content

Commit de225d9

Browse files
vertex451vertex451
committed
chore: simplified watcher logic by removing function as a param
1 parent baea1e4 commit de225d9

File tree

3 files changed

+93
-85
lines changed

3 files changed

+93
-85
lines changed

‎listener/reconciler/kcp/config_watcher.go‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,25 @@ type VirtualWorkspaceConfigManager interface {
1313
LoadConfig(configPath string) (*VirtualWorkspacesConfig, error)
1414
}
1515

16+
// VirtualWorkspaceConfigReconciler interface for reconciling virtual workspace configurations
17+
type VirtualWorkspaceConfigReconciler interface {
18+
ReconcileConfig(ctx context.Context, config *VirtualWorkspacesConfig) error
19+
}
20+
1621
// ConfigWatcher watches the virtual workspaces configuration file for changes
1722
type ConfigWatcher struct {
1823
fileWatcher *watcher.FileWatcher
1924
virtualWSManager VirtualWorkspaceConfigManager
25+
reconciler VirtualWorkspaceConfigReconciler
2026
log *logger.Logger
21-
changeHandler func(*VirtualWorkspacesConfig)
27+
ctx context.Context
2228
}
2329

2430
// NewConfigWatcher creates a new config file watcher
25-
func NewConfigWatcher(virtualWSManager VirtualWorkspaceConfigManager, log *logger.Logger) (*ConfigWatcher, error) {
31+
func NewConfigWatcher(virtualWSManager VirtualWorkspaceConfigManager, reconciler VirtualWorkspaceConfigReconciler, log *logger.Logger) (*ConfigWatcher, error) {
2632
c := &ConfigWatcher{
2733
virtualWSManager: virtualWSManager,
34+
reconciler: reconciler,
2835
log: log,
2936
}
3037

@@ -38,18 +45,17 @@ func NewConfigWatcher(virtualWSManager VirtualWorkspaceConfigManager, log *logge
3845
}
3946

4047
// Watch starts watching the configuration file and blocks until context is cancelled
41-
func (c *ConfigWatcher) Watch(ctx context.Context, configPath string, changeHandler func(*VirtualWorkspacesConfig)) error {
42-
// Store change handler for use in event callbacks
43-
c.changeHandler = changeHandler
44-
45-
// Load initial configuration
46-
if configPath != "" {
47-
if err := c.loadAndNotify(configPath); err != nil {
48-
c.log.Error().Err(err).Msg("failed to load initial virtual workspaces config")
49-
}
48+
func (c *ConfigWatcher) Watch(ctx context.Context, configPath string) error {
49+
if configPath == "" {
50+
return nil
51+
}
52+
53+
c.ctx = ctx
54+
55+
if err := c.loadAndNotify(configPath); err != nil {
56+
c.log.Error().Err(err).Msg("failed to load initial virtual workspaces config")
5057
}
5158

52-
// Watch optional configuration file with 500ms debouncing
5359
return c.fileWatcher.WatchOptionalFile(ctx, configPath, 500)
5460
}
5561

@@ -65,7 +71,7 @@ func (c *ConfigWatcher) OnFileDeleted(filepath string) {
6571
c.log.Warn().Str("configPath", filepath).Msg("virtual workspaces config file deleted")
6672
}
6773

68-
// loadAndNotify loads the config and notifies the change handler
74+
// loadAndNotify loads the config and reconciles it
6975
func (c *ConfigWatcher) loadAndNotify(configPath string) error {
7076
config, err := c.virtualWSManager.LoadConfig(configPath)
7177
if err != nil {
@@ -74,8 +80,8 @@ func (c *ConfigWatcher) loadAndNotify(configPath string) error {
7480

7581
c.log.Info().Int("virtualWorkspaces", len(config.VirtualWorkspaces)).Msg("loaded virtual workspaces config")
7682

77-
if c.changeHandler != nil {
78-
c.changeHandler(config)
83+
if err := c.reconciler.ReconcileConfig(c.ctx, config); err != nil {
84+
c.log.Error().Err(err).Msg("failed to reconcile virtual workspaces config")
7985
}
8086
return nil
8187
}

‎listener/reconciler/kcp/config_watcher_test.go‎

Lines changed: 70 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,18 @@ func (m *MockVirtualWorkspaceConfigManager) LoadConfig(configPath string) (*Virt
2323
return &VirtualWorkspacesConfig{}, nil
2424
}
2525

26+
// MockVirtualWorkspaceReconciler for testing
27+
type MockVirtualWorkspaceReconciler struct {
28+
ReconcileConfigFunc func(ctx context.Context, config *VirtualWorkspacesConfig) error
29+
}
30+
31+
func (m *MockVirtualWorkspaceReconciler) ReconcileConfig(ctx context.Context, config *VirtualWorkspacesConfig) error {
32+
if m.ReconcileConfigFunc != nil {
33+
return m.ReconcileConfigFunc(ctx, config)
34+
}
35+
return nil
36+
}
37+
2638
func TestNewConfigWatcher(t *testing.T) {
2739
tests := []struct {
2840
name string
@@ -37,8 +49,9 @@ func TestNewConfigWatcher(t *testing.T) {
3749
for _, tt := range tests {
3850
t.Run(tt.name, func(t *testing.T) {
3951
virtualWSManager := &MockVirtualWorkspaceConfigManager{}
52+
reconciler := &MockVirtualWorkspaceReconciler{}
4053

41-
watcher, err := NewConfigWatcher(virtualWSManager, testlogger.New().Logger)
54+
watcher, err := NewConfigWatcher(virtualWSManager, reconciler, testlogger.New().Logger)
4255

4356
if tt.expectError {
4457
assert.Error(t, err)
@@ -47,6 +60,7 @@ func TestNewConfigWatcher(t *testing.T) {
4760
assert.NoError(t, err)
4861
assert.NotNil(t, watcher)
4962
assert.Equal(t, virtualWSManager, watcher.virtualWSManager)
63+
assert.Equal(t, reconciler, watcher.reconciler)
5064
assert.Equal(t, testlogger.New().Logger, watcher.log)
5165
assert.NotNil(t, watcher.fileWatcher)
5266
}
@@ -56,10 +70,10 @@ func TestNewConfigWatcher(t *testing.T) {
5670

5771
func TestConfigWatcher_OnFileChanged(t *testing.T) {
5872
tests := []struct {
59-
name string
60-
filepath string
61-
loadConfigFunc func(configPath string) (*VirtualWorkspacesConfig, error)
62-
expectHandlerCall bool
73+
name string
74+
filepath string
75+
loadConfigFunc func(configPath string) (*VirtualWorkspacesConfig, error)
76+
expectReconcilerCall bool
6377
}{
6478
{
6579
name: "successful_file_change",
@@ -71,15 +85,15 @@ func TestConfigWatcher_OnFileChanged(t *testing.T) {
7185
},
7286
}, nil
7387
},
74-
expectHandlerCall: true,
88+
expectReconcilerCall: true,
7589
},
7690
{
7791
name: "failed_config_load",
7892
filepath: "/test/config.yaml",
7993
loadConfigFunc: func(configPath string) (*VirtualWorkspacesConfig, error) {
8094
return nil, errors.New("failed to load config")
8195
},
82-
expectHandlerCall: false,
96+
expectReconcilerCall: false,
8397
},
8498
}
8599

@@ -89,49 +103,51 @@ func TestConfigWatcher_OnFileChanged(t *testing.T) {
89103
LoadConfigFunc: tt.loadConfigFunc,
90104
}
91105

92-
watcher, err := NewConfigWatcher(virtualWSManager, testlogger.New().Logger)
93-
require.NoError(t, err)
94-
95-
// Track change handler calls
96-
var handlerCalled bool
106+
var reconcilerCalled bool
97107
var receivedConfig *VirtualWorkspacesConfig
98-
changeHandler := func(config *VirtualWorkspacesConfig) {
99-
handlerCalled = true
100-
receivedConfig = config
108+
reconciler := &MockVirtualWorkspaceReconciler{
109+
ReconcileConfigFunc: func(ctx context.Context, config *VirtualWorkspacesConfig) error {
110+
reconcilerCalled = true
111+
receivedConfig = config
112+
return nil
113+
},
101114
}
102-
watcher.changeHandler = changeHandler
115+
116+
watcher, err := NewConfigWatcher(virtualWSManager, reconciler, testlogger.New().Logger)
117+
require.NoError(t, err)
118+
watcher.ctx = context.Background()
103119

104120
watcher.OnFileChanged(tt.filepath)
105121

106-
if tt.expectHandlerCall {
107-
assert.True(t, handlerCalled)
122+
if tt.expectReconcilerCall {
123+
assert.True(t, reconcilerCalled)
108124
assert.NotNil(t, receivedConfig)
109125
assert.Equal(t, 1, len(receivedConfig.VirtualWorkspaces))
110126
assert.Equal(t, "test-ws", receivedConfig.VirtualWorkspaces[0].Name)
111127
} else {
112-
assert.False(t, handlerCalled)
128+
assert.False(t, reconcilerCalled)
113129
}
114130
})
115131
}
116132
}
117133

118134
func TestConfigWatcher_OnFileDeleted(t *testing.T) {
119135
virtualWSManager := &MockVirtualWorkspaceConfigManager{}
136+
reconciler := &MockVirtualWorkspaceReconciler{}
120137

121-
watcher, err := NewConfigWatcher(virtualWSManager, testlogger.New().Logger)
138+
watcher, err := NewConfigWatcher(virtualWSManager, reconciler, testlogger.New().Logger)
122139
require.NoError(t, err)
123140

124-
// Should not panic or error
125141
watcher.OnFileDeleted("/test/config.yaml")
126142
}
127143

128144
func TestConfigWatcher_LoadAndNotify(t *testing.T) {
129145
tests := []struct {
130-
name string
131-
configPath string
132-
loadConfigFunc func(configPath string) (*VirtualWorkspacesConfig, error)
133-
expectError bool
134-
expectCall bool
146+
name string
147+
configPath string
148+
loadConfigFunc func(configPath string) (*VirtualWorkspacesConfig, error)
149+
expectError bool
150+
expectReconcilerCall bool
135151
}{
136152
{
137153
name: "successful_load_and_notify",
@@ -144,26 +160,17 @@ func TestConfigWatcher_LoadAndNotify(t *testing.T) {
144160
},
145161
}, nil
146162
},
147-
expectError: false,
148-
expectCall: true,
163+
expectError: false,
164+
expectReconcilerCall: true,
149165
},
150166
{
151167
name: "failed_config_load",
152168
configPath: "/test/config.yaml",
153169
loadConfigFunc: func(configPath string) (*VirtualWorkspacesConfig, error) {
154170
return nil, errors.New("config load error")
155171
},
156-
expectError: true,
157-
expectCall: false,
158-
},
159-
{
160-
name: "no_change_handler",
161-
configPath: "/test/config.yaml",
162-
loadConfigFunc: func(configPath string) (*VirtualWorkspacesConfig, error) {
163-
return &VirtualWorkspacesConfig{}, nil
164-
},
165-
expectError: false,
166-
expectCall: false,
172+
expectError: true,
173+
expectReconcilerCall: false,
167174
},
168175
}
169176

@@ -173,20 +180,20 @@ func TestConfigWatcher_LoadAndNotify(t *testing.T) {
173180
LoadConfigFunc: tt.loadConfigFunc,
174181
}
175182

176-
watcher, err := NewConfigWatcher(virtualWSManager, testlogger.New().Logger)
177-
require.NoError(t, err)
178-
179-
// Track change handler calls
180-
var handlerCalled bool
183+
var reconcilerCalled bool
181184
var receivedConfig *VirtualWorkspacesConfig
182-
if tt.name != "no_change_handler" {
183-
changeHandler := func(config *VirtualWorkspacesConfig) {
184-
handlerCalled = true
185+
reconciler := &MockVirtualWorkspaceReconciler{
186+
ReconcileConfigFunc: func(ctx context.Context, config *VirtualWorkspacesConfig) error {
187+
reconcilerCalled = true
185188
receivedConfig = config
186-
}
187-
watcher.changeHandler = changeHandler
189+
return nil
190+
},
188191
}
189192

193+
watcher, err := NewConfigWatcher(virtualWSManager, reconciler, testlogger.New().Logger)
194+
require.NoError(t, err)
195+
watcher.ctx = context.Background()
196+
190197
err = watcher.loadAndNotify(tt.configPath)
191198

192199
if tt.expectError {
@@ -195,14 +202,14 @@ func TestConfigWatcher_LoadAndNotify(t *testing.T) {
195202
assert.NoError(t, err)
196203
}
197204

198-
if tt.expectCall {
199-
assert.True(t, handlerCalled)
205+
if tt.expectReconcilerCall {
206+
assert.True(t, reconcilerCalled)
200207
assert.NotNil(t, receivedConfig)
201208
if tt.name == "successful_load_and_notify" {
202209
assert.Equal(t, 2, len(receivedConfig.VirtualWorkspaces))
203210
}
204211
} else {
205-
assert.False(t, handlerCalled)
212+
assert.False(t, reconcilerCalled)
206213
}
207214
})
208215
}
@@ -215,21 +222,22 @@ func TestConfigWatcher_Watch_EmptyPath(t *testing.T) {
215222
},
216223
}
217224

218-
watcher, err := NewConfigWatcher(virtualWSManager, testlogger.New().Logger)
225+
var reconcilerCalled bool
226+
reconciler := &MockVirtualWorkspaceReconciler{
227+
ReconcileConfigFunc: func(ctx context.Context, config *VirtualWorkspacesConfig) error {
228+
reconcilerCalled = true
229+
return nil
230+
},
231+
}
232+
233+
watcher, err := NewConfigWatcher(virtualWSManager, reconciler, testlogger.New().Logger)
219234
require.NoError(t, err)
220235

221236
ctx, cancel := context.WithTimeout(t.Context(), common.ShortTimeout)
222237
defer cancel()
223238

224-
var handlerCalled bool
225-
changeHandler := func(config *VirtualWorkspacesConfig) {
226-
handlerCalled = true
227-
}
228-
229-
// Test with empty config path - should not try to load initial config
230-
err = watcher.Watch(ctx, "", changeHandler)
239+
err = watcher.Watch(ctx, "")
231240

232-
// Should complete gracefully without error since graceful termination is not an error
233241
assert.NoError(t, err)
234-
assert.False(t, handlerCalled) // Should not call handler for empty path initial load
242+
assert.False(t, reconcilerCalled)
235243
}

‎listener/reconciler/kcp/reconciler.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func NewKCPReconciler(
8181
log,
8282
)
8383

84-
configWatcher, err := NewConfigWatcher(virtualWSManager, log)
84+
configWatcher, err := NewConfigWatcher(virtualWSManager, virtualWorkspaceReconciler, log)
8585
if err != nil {
8686
log.Error().Err(err).Msg("failed to create config watcher")
8787
return nil, err
@@ -137,11 +137,5 @@ func (r *KCPReconciler) StartVirtualWorkspaceWatching(ctx context.Context, confi
137137

138138
r.log.Info().Str("configPath", configPath).Msg("starting virtual workspace configuration watching")
139139

140-
// Start config watcher with a wrapper function
141-
changeHandler := func(config *VirtualWorkspacesConfig) {
142-
if err := r.virtualWorkspaceReconciler.ReconcileConfig(ctx, config); err != nil {
143-
r.log.Error().Err(err).Msg("failed to reconcile virtual workspaces config")
144-
}
145-
}
146-
return r.configWatcher.Watch(ctx, configPath, changeHandler)
140+
return r.configWatcher.Watch(ctx, configPath)
147141
}

0 commit comments

Comments
 (0)