Skip to content

Commit f2d17ee

Browse files
Random-LiuRandom-Liu
committed
Do not import plugins unnecessarily.
Signed-off-by: Lantao Liu <[email protected]>
1 parent 975dc71 commit f2d17ee

File tree

2 files changed

+67
-37
lines changed

2 files changed

+67
-37
lines changed

‎cmd/options/options.go

+29-14
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,7 @@ import (
2525

2626
"github.com/spf13/pflag"
2727

28-
"k8s.io/node-problem-detector/pkg/custompluginmonitor"
2928
"k8s.io/node-problem-detector/pkg/problemdaemon"
30-
"k8s.io/node-problem-detector/pkg/systemlogmonitor"
3129
"k8s.io/node-problem-detector/pkg/types"
3230
)
3331

@@ -80,7 +78,11 @@ type NodeProblemDetectorOptions struct {
8078
}
8179

8280
func NewNodeProblemDetectorOptions() *NodeProblemDetectorOptions {
83-
return &NodeProblemDetectorOptions{MonitorConfigPaths: types.ProblemDaemonConfigPathMap{}}
81+
npdo := &NodeProblemDetectorOptions{MonitorConfigPaths: types.ProblemDaemonConfigPathMap{}}
82+
for _, problemDaemonName := range problemdaemon.GetProblemDaemonNames() {
83+
npdo.MonitorConfigPaths[problemDaemonName] = &[]string{}
84+
}
85+
return npdo
8486
}
8587

8688
// AddFlags adds node problem detector command line options to pflag.
@@ -108,7 +110,6 @@ func (npdo *NodeProblemDetectorOptions) AddFlags(fs *pflag.FlagSet) {
108110
"127.0.0.1", "The address to bind the Prometheus scrape endpoint.")
109111

110112
for _, problemDaemonName := range problemdaemon.GetProblemDaemonNames() {
111-
npdo.MonitorConfigPaths[problemDaemonName] = &[]string{}
112113
fs.StringSliceVar(
113114
npdo.MonitorConfigPaths[problemDaemonName],
114115
"config."+string(problemDaemonName),
@@ -142,34 +143,48 @@ func (npdo *NodeProblemDetectorOptions) ValidOrDie() {
142143
}
143144
}
144145

146+
// Plugin names for custom plugin monitor and system log monitor.
147+
// Hard code them here to:
148+
// 1) Handle deprecated flags for --system-log-monitors and --custom-plugin-monitors.
149+
// 2) Avoid direct dependencies to packages in those plugins, so that those plugins
150+
// can be disabled at compile time.
151+
const (
152+
customPluginMonitorName = "custom-plugin-monitor"
153+
systemLogMonitorName = "system-log-monitor"
154+
)
155+
145156
// SetConfigFromDeprecatedOptionsOrDie sets NPD option using deprecated options.
146157
func (npdo *NodeProblemDetectorOptions) SetConfigFromDeprecatedOptionsOrDie() {
147158
if len(npdo.SystemLogMonitorConfigPaths) != 0 {
148-
if npdo.MonitorConfigPaths[systemlogmonitor.SystemLogMonitorName] == nil {
149-
npdo.MonitorConfigPaths[systemlogmonitor.SystemLogMonitorName] = &[]string{}
159+
if npdo.MonitorConfigPaths[systemLogMonitorName] == nil {
160+
// As long as the problem daemon is registered, MonitorConfigPaths should
161+
// not be nil.
162+
panic("System log monitor is not supported")
150163
}
151164

152-
if len(*npdo.MonitorConfigPaths[systemlogmonitor.SystemLogMonitorName]) != 0 {
165+
if len(*npdo.MonitorConfigPaths[systemLogMonitorName]) != 0 {
153166
panic("Option --system-log-monitors is deprecated in favor of --config.system-log-monitor. They cannot be set at the same time.")
154167
}
155168

156-
*npdo.MonitorConfigPaths[systemlogmonitor.SystemLogMonitorName] = append(
157-
*npdo.MonitorConfigPaths[systemlogmonitor.SystemLogMonitorName],
169+
*npdo.MonitorConfigPaths[systemLogMonitorName] = append(
170+
*npdo.MonitorConfigPaths[systemLogMonitorName],
158171
npdo.SystemLogMonitorConfigPaths...)
159172
npdo.SystemLogMonitorConfigPaths = []string{}
160173
}
161174

162175
if len(npdo.CustomPluginMonitorConfigPaths) != 0 {
163-
if npdo.MonitorConfigPaths[custompluginmonitor.CustomPluginMonitorName] == nil {
164-
npdo.MonitorConfigPaths[custompluginmonitor.CustomPluginMonitorName] = &[]string{}
176+
if npdo.MonitorConfigPaths[customPluginMonitorName] == nil {
177+
// As long as the problem daemon is registered, MonitorConfigPaths should
178+
// not be nil.
179+
panic("Custom plugin monitor is not supported")
165180
}
166181

167-
if len(*npdo.MonitorConfigPaths[custompluginmonitor.CustomPluginMonitorName]) != 0 {
182+
if len(*npdo.MonitorConfigPaths[customPluginMonitorName]) != 0 {
168183
panic("Option --custom-plugin-monitors is deprecated in favor of --config.custom-plugin-monitor. They cannot be set at the same time.")
169184
}
170185

171-
*npdo.MonitorConfigPaths[custompluginmonitor.CustomPluginMonitorName] = append(
172-
*npdo.MonitorConfigPaths[custompluginmonitor.CustomPluginMonitorName],
186+
*npdo.MonitorConfigPaths[customPluginMonitorName] = append(
187+
*npdo.MonitorConfigPaths[customPluginMonitorName],
173188
npdo.CustomPluginMonitorConfigPaths...)
174189
npdo.CustomPluginMonitorConfigPaths = []string{}
175190
}

‎cmd/options/options_test.go

+38-23
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ import (
2323

2424
"github.com/stretchr/testify/assert"
2525

26-
"k8s.io/node-problem-detector/pkg/custompluginmonitor"
27-
"k8s.io/node-problem-detector/pkg/systemlogmonitor"
2826
"k8s.io/node-problem-detector/pkg/types"
2927
)
3028

@@ -255,15 +253,15 @@ func TestSetConfigFromDeprecatedOptionsOrDie(t *testing.T) {
255253
name: "no deprecated options",
256254
orig: NodeProblemDetectorOptions{
257255
MonitorConfigPaths: types.ProblemDaemonConfigPathMap{
258-
systemlogmonitor.SystemLogMonitorName: &[]string{"config-a", "config-b"},
259-
custompluginmonitor.CustomPluginMonitorName: &[]string{"config-c", "config-d"},
256+
systemLogMonitorName: &[]string{"config-a", "config-b"},
257+
customPluginMonitorName: &[]string{"config-c", "config-d"},
260258
},
261259
},
262260
expectPanic: false,
263261
wanted: NodeProblemDetectorOptions{
264262
MonitorConfigPaths: types.ProblemDaemonConfigPathMap{
265-
systemlogmonitor.SystemLogMonitorName: &[]string{"config-a", "config-b"},
266-
custompluginmonitor.CustomPluginMonitorName: &[]string{"config-c", "config-d"},
263+
systemLogMonitorName: &[]string{"config-a", "config-b"},
264+
customPluginMonitorName: &[]string{"config-c", "config-d"},
267265
},
268266
},
269267
},
@@ -272,13 +270,16 @@ func TestSetConfigFromDeprecatedOptionsOrDie(t *testing.T) {
272270
orig: NodeProblemDetectorOptions{
273271
SystemLogMonitorConfigPaths: []string{"config-a", "config-b"},
274272
CustomPluginMonitorConfigPaths: []string{"config-c", "config-d"},
275-
MonitorConfigPaths: types.ProblemDaemonConfigPathMap{},
273+
MonitorConfigPaths: types.ProblemDaemonConfigPathMap{
274+
customPluginMonitorName: &[]string{},
275+
systemLogMonitorName: &[]string{},
276+
},
276277
},
277278
expectPanic: false,
278279
wanted: NodeProblemDetectorOptions{
279280
MonitorConfigPaths: types.ProblemDaemonConfigPathMap{
280-
systemlogmonitor.SystemLogMonitorName: &[]string{"config-a", "config-b"},
281-
custompluginmonitor.CustomPluginMonitorName: &[]string{"config-c", "config-d"},
281+
systemLogMonitorName: &[]string{"config-a", "config-b"},
282+
customPluginMonitorName: &[]string{"config-c", "config-d"},
282283
},
283284
},
284285
},
@@ -287,14 +288,15 @@ func TestSetConfigFromDeprecatedOptionsOrDie(t *testing.T) {
287288
orig: NodeProblemDetectorOptions{
288289
SystemLogMonitorConfigPaths: []string{"config-a", "config-b"},
289290
MonitorConfigPaths: types.ProblemDaemonConfigPathMap{
290-
custompluginmonitor.CustomPluginMonitorName: &[]string{"config-c", "config-d"},
291+
customPluginMonitorName: &[]string{"config-c", "config-d"},
292+
systemLogMonitorName: &[]string{},
291293
},
292294
},
293295
expectPanic: false,
294296
wanted: NodeProblemDetectorOptions{
295297
MonitorConfigPaths: types.ProblemDaemonConfigPathMap{
296-
systemlogmonitor.SystemLogMonitorName: &[]string{"config-a", "config-b"},
297-
custompluginmonitor.CustomPluginMonitorName: &[]string{"config-c", "config-d"},
298+
systemLogMonitorName: &[]string{"config-a", "config-b"},
299+
customPluginMonitorName: &[]string{"config-c", "config-d"},
298300
},
299301
},
300302
},
@@ -303,14 +305,15 @@ func TestSetConfigFromDeprecatedOptionsOrDie(t *testing.T) {
303305
orig: NodeProblemDetectorOptions{
304306
CustomPluginMonitorConfigPaths: []string{"config-a", "config-b"},
305307
MonitorConfigPaths: types.ProblemDaemonConfigPathMap{
306-
systemlogmonitor.SystemLogMonitorName: &[]string{"config-c", "config-d"},
308+
customPluginMonitorName: &[]string{},
309+
systemLogMonitorName: &[]string{"config-c", "config-d"},
307310
},
308311
},
309312
expectPanic: false,
310313
wanted: NodeProblemDetectorOptions{
311314
MonitorConfigPaths: types.ProblemDaemonConfigPathMap{
312-
systemlogmonitor.SystemLogMonitorName: &[]string{"config-c", "config-d"},
313-
custompluginmonitor.CustomPluginMonitorName: &[]string{"config-a", "config-b"},
315+
systemLogMonitorName: &[]string{"config-c", "config-d"},
316+
customPluginMonitorName: &[]string{"config-a", "config-b"},
314317
},
315318
},
316319
},
@@ -319,30 +322,42 @@ func TestSetConfigFromDeprecatedOptionsOrDie(t *testing.T) {
319322
orig: NodeProblemDetectorOptions{
320323
SystemLogMonitorConfigPaths: []string{"config-a"},
321324
MonitorConfigPaths: types.ProblemDaemonConfigPathMap{
322-
systemlogmonitor.SystemLogMonitorName: &[]string{"config-b"},
325+
systemLogMonitorName: &[]string{"config-b"},
323326
},
324327
},
325328
expectPanic: true,
326-
wanted: NodeProblemDetectorOptions{
329+
},
330+
{
331+
name: "using deprecated & new options on CustomPluginMonitor",
332+
orig: NodeProblemDetectorOptions{
333+
CustomPluginMonitorConfigPaths: []string{"config-a"},
327334
MonitorConfigPaths: types.ProblemDaemonConfigPathMap{
328-
systemlogmonitor.SystemLogMonitorName: &[]string{"config-b"},
335+
customPluginMonitorName: &[]string{"config-b"},
329336
},
330337
},
338+
expectPanic: true,
331339
},
332340
{
333-
name: "using deprecated & new options on CustomPluginMonitor",
341+
name: "using deprecated options when SystemLogMonitor is not registered",
334342
orig: NodeProblemDetectorOptions{
335-
CustomPluginMonitorConfigPaths: []string{"config-a"},
343+
SystemLogMonitorConfigPaths: []string{"config-a"},
344+
CustomPluginMonitorConfigPaths: []string{"config-b"},
336345
MonitorConfigPaths: types.ProblemDaemonConfigPathMap{
337-
custompluginmonitor.CustomPluginMonitorName: &[]string{"config-b"},
346+
customPluginMonitorName: &[]string{},
338347
},
339348
},
340349
expectPanic: true,
341-
wanted: NodeProblemDetectorOptions{
350+
},
351+
{
352+
name: "using deprecated options when CustomPluginMonitor is not registered",
353+
orig: NodeProblemDetectorOptions{
354+
SystemLogMonitorConfigPaths: []string{"config-a"},
355+
CustomPluginMonitorConfigPaths: []string{"config-b"},
342356
MonitorConfigPaths: types.ProblemDaemonConfigPathMap{
343-
custompluginmonitor.CustomPluginMonitorName: &[]string{"config-b"},
357+
systemLogMonitorName: &[]string{},
344358
},
345359
},
360+
expectPanic: true,
346361
},
347362
}
348363

0 commit comments

Comments
 (0)