Skip to content

Commit 7b3d026

Browse files
committed
fix issues found from review
Signed-off-by: Davanum Srinivas <[email protected]>
1 parent c98a4d9 commit 7b3d026

File tree

6 files changed

+33
-25
lines changed

6 files changed

+33
-25
lines changed

health-monitors/syslog-health-monitor/pkg/xid/parser/sidecar_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,13 @@ func TestSidecarParser_Parse(t *testing.T) {
118118

119119
if tc.mockStatusCode == 200 && tc.mockResponse != nil {
120120
w.Header().Set("Content-Type", "application/json")
121-
_ = json.NewEncoder(w).Encode(tc.mockResponse)
121+
if err := json.NewEncoder(w).Encode(tc.mockResponse); err != nil {
122+
t.Fatalf("encode response: %v", err)
123+
}
122124
} else if tc.mockStatusCode != 200 {
123-
_, _ = w.Write([]byte("Server error"))
125+
if _, err := w.Write([]byte("Server error")); err != nil {
126+
t.Fatalf("write error response: %v", err)
127+
}
124128
}
125129
}))
126130
defer server.Close()

node-drainer/pkg/config/config.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,10 +167,16 @@ func NewDatabaseConfig(databaseClientCertMountPath string) (config.DatabaseConfi
167167
return config.NewDatabaseConfigFromEnv()
168168
}
169169

170-
// NewTokenConfig creates a token configuration from environment config
170+
// NewTokenConfig is DEPRECATED and should not be used.
171+
// This function hardcoded ClientName="node-draining-module", which caused resume token
172+
// lookup failures because LoadEnvConfig uses ClientName="node-drainer".
173+
// Instead, use config.TokenConfigFromEnv("node-drainer") directly like other modules.
174+
//
175+
// Deprecated: Use config.TokenConfigFromEnv("node-drainer") instead.
171176
func NewTokenConfig(envConfig *EnvConfig) client.TokenConfig {
177+
// Return config with the CORRECT ClientName to match what's used for token storage
172178
return client.TokenConfig{
173-
ClientName: "node-draining-module",
179+
ClientName: "node-drainer", // Fixed: was "node-draining-module"
174180
TokenDatabase: envConfig.TokenDatabase,
175181
TokenCollection: envConfig.TokenCollection,
176182
}

node-drainer/pkg/initializer/init.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,10 @@ type Components struct {
5555
func InitializeAll(ctx context.Context, params InitializationParams) (*Components, error) {
5656
slog.Info("Starting node drainer initialization")
5757

58-
// Load environment configuration using the new unified system
59-
envConfig, err := config.LoadEnvConfig()
58+
// Load token configuration - preserves ClientName="node-drainer" for resume token lookups
59+
tokenConfig, err := sdkconfig.TokenConfigFromEnv("node-drainer")
6060
if err != nil {
61-
return nil, fmt.Errorf("failed to load environment config: %w", err)
61+
return nil, fmt.Errorf("failed to load token configuration: %w", err)
6262
}
6363

6464
// Load datastore configuration using the new unified system
@@ -70,7 +70,6 @@ func InitializeAll(ctx context.Context, params InitializationParams) (*Component
7070
// Convert to legacy DatabaseConfig interface for compatibility with existing factory
7171
// Pass the certificate mount path to the adapter to handle path resolution at runtime
7272
databaseConfig := adapter.ConvertDataStoreConfigToLegacyWithCertPath(dsConfig, params.DatabaseClientCertMountPath)
73-
tokenConfig := config.NewTokenConfig(envConfig)
7473
pipeline := config.NewQuarantinePipeline()
7574

7675
tomlCfg, err := config.LoadTomlConfig(params.TomlConfigPath)
@@ -95,7 +94,16 @@ func InitializeAll(ctx context.Context, params InitializationParams) (*Component
9594
}
9695

9796
stateManager := initializeStateManager(clientSet)
98-
reconcilerCfg := createReconcilerConfig(*tomlCfg, databaseConfig, tokenConfig, pipeline, stateManager)
97+
98+
// Convert store-client TokenConfig to client.TokenConfig type
99+
// IMPORTANT: Preserves ClientName="node-drainer" for resume token lookups
100+
clientTokenConfig := client.TokenConfig{
101+
ClientName: tokenConfig.ClientName,
102+
TokenDatabase: tokenConfig.TokenDatabase,
103+
TokenCollection: tokenConfig.TokenCollection,
104+
}
105+
106+
reconcilerCfg := createReconcilerConfig(*tomlCfg, databaseConfig, clientTokenConfig, pipeline, stateManager)
99107

100108
// Create client factory and database client
101109
clientFactory := factory.NewClientFactory(databaseConfig)

scripts/validate-gomod.sh

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -191,11 +191,8 @@ check_mongodb_imports() {
191191

192192
# Check for direct (non-indirect) MongoDB driver dependencies
193193
local mongodb_deps
194-
if ! mongodb_deps=$(cd "$gomod_dir" && go list -m -f '{{if not .Indirect}}{{.Path}}{{end}}' all 2>/dev/null | grep "^go.mongodb.org/mongo-driver" || true); then
195-
# go list failed, skip this check
196-
return 0
197-
fi
198-
194+
mongodb_deps=$(cd "$gomod_dir" && go list -m -f '{{if not .Indirect}}{{.Path}}{{end}}' all 2>/dev/null | grep "^go.mongodb.org/mongo-driver" || true)
195+
199196
if [[ -n "$mongodb_deps" ]]; then
200197
print_error " ✗ Direct MongoDB driver dependency detected in go.mod:"
201198
print_error " $mongodb_deps"

store-client/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ err := dbClient.WithTransaction(ctx, func(sessCtx client.SessionContext) error {
200200

201201
## Architecture
202202

203-
```
203+
```text
204204
┌─────────────────┐
205205
│ Your Module │
206206
└────────┬────────┘

store-client/pkg/datastore/providers/mongodb/adapter.go

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -142,16 +142,9 @@ func (a *AdaptedMongoStore) Provider() datastore.DataStoreProvider {
142142
func (a *AdaptedMongoStore) CreateChangeStreamWatcher(ctx context.Context, clientName string,
143143
pipeline interface{}) (datastore.ChangeStreamWatcher, error) {
144144
// Use our existing factory to create a change stream watcher
145-
tokenConfig := client.TokenConfig{
146-
ClientName: clientName,
147-
TokenDatabase: a.config.Connection.Database,
148-
TokenCollection: "ResumeTokens", // Default token collection
149-
}
150-
151-
// Check if token collection is specified in config
152-
if tokenColl := a.config.Options["tokenCollection"]; tokenColl != "" {
153-
tokenConfig.TokenCollection = tokenColl
154-
}
145+
// Note: Token configuration is loaded from environment variables by the factory
146+
// via config.TokenConfigFromEnv(clientName). To customize token collection,
147+
// set the MONGODB_TOKEN_COLLECTION_NAME environment variable.
155148

156149
// CRITICAL: Pass the existing databaseClient to avoid creating duplicate clients
157150
watcher, err := a.factory.CreateChangeStreamWatcher(ctx, a.databaseClient, clientName, pipeline)

0 commit comments

Comments
 (0)