From 8f0fd1a3a4a7dee9a65e94f317d9bb7055a5715d Mon Sep 17 00:00:00 2001 From: Xuewei Zhang Date: Thu, 12 Sep 2019 18:28:36 -0700 Subject: [PATCH] Allow problem daemon plugins to define their command line options --- .../node_problem_detector.go | 5 +- cmd/options/options.go | 103 +------ cmd/options/options_test.go | 266 +----------------- .../custom_plugin_monitor.go | 24 +- pkg/custompluginmonitor/types/options.go | 40 +++ pkg/problemdaemon/problem_daemon.go | 21 +- pkg/problemdaemon/problem_daemon_test.go | 21 +- pkg/systemlogmonitor/log_monitor.go | 24 +- pkg/systemlogmonitor/options.go | 39 +++ .../system_stats_monitor.go | 19 +- pkg/systemstatsmonitor/types/options.go | 32 +++ pkg/types/types.go | 15 +- 12 files changed, 204 insertions(+), 405 deletions(-) create mode 100644 pkg/custompluginmonitor/types/options.go create mode 100644 pkg/systemlogmonitor/options.go create mode 100644 pkg/systemstatsmonitor/types/options.go diff --git a/cmd/nodeproblemdetector/node_problem_detector.go b/cmd/nodeproblemdetector/node_problem_detector.go index 42fc708a4..6426bb0da 100644 --- a/cmd/nodeproblemdetector/node_problem_detector.go +++ b/cmd/nodeproblemdetector/node_problem_detector.go @@ -35,7 +35,7 @@ import ( ) func main() { - npdo := options.NewNodeProblemDetectorOptions() + npdo := &options.NodeProblemDetectorOptions{} npdo.AddFlags(pflag.CommandLine) pflag.Parse() @@ -46,11 +46,10 @@ func main() { } npdo.SetNodeNameOrDie() - npdo.SetConfigFromDeprecatedOptionsOrDie() npdo.ValidOrDie() // Initialize problem daemons. - problemDaemons := problemdaemon.NewProblemDaemons(npdo.MonitorConfigPaths) + problemDaemons := problemdaemon.NewProblemDaemons() if len(problemDaemons) == 0 { glog.Fatalf("No problem daemon is configured") } diff --git a/cmd/options/options.go b/cmd/options/options.go index acc8327d7..32a480f21 100644 --- a/cmd/options/options.go +++ b/cmd/options/options.go @@ -28,7 +28,6 @@ import ( "k8s.io/node-problem-detector/pkg/exporters" "k8s.io/node-problem-detector/pkg/problemdaemon" - "k8s.io/node-problem-detector/pkg/types" ) // NodeProblemDetectorOptions contains node problem detector command line and application options. @@ -64,44 +63,14 @@ type NodeProblemDetectorOptions struct { // PrometheusServerAddress is the address to bind the Prometheus scrape endpoint. PrometheusServerAddress string - // problem daemon options - - // SystemLogMonitorConfigPaths specifies the list of paths to system log monitor configuration - // files. - // SystemLogMonitorConfigPaths is used by the deprecated option --system-log-monitors. The new - // option --config.system-log-monitor will stored the config file paths in MonitorConfigPaths. - SystemLogMonitorConfigPaths []string - // CustomPluginMonitorConfigPaths specifies the list of paths to custom plugin monitor configuration - // files. - // CustomPluginMonitorConfigPaths is used by the deprecated option --custom-plugin-monitors. The - // new option --config.custom-plugin-monitor will stored the config file paths in MonitorConfigPaths. - CustomPluginMonitorConfigPaths []string - // MonitorConfigPaths specifies the list of paths to configuration files for each monitor. - MonitorConfigPaths types.ProblemDaemonConfigPathMap - // application options // NodeName is the node name used to communicate with Kubernetes ApiServer. NodeName string } -func NewNodeProblemDetectorOptions() *NodeProblemDetectorOptions { - npdo := &NodeProblemDetectorOptions{MonitorConfigPaths: types.ProblemDaemonConfigPathMap{}} - - for _, problemDaemonName := range problemdaemon.GetProblemDaemonNames() { - npdo.MonitorConfigPaths[problemDaemonName] = &[]string{} - } - return npdo -} - // AddFlags adds node problem detector command line options to pflag. func (npdo *NodeProblemDetectorOptions) AddFlags(fs *pflag.FlagSet) { - fs.StringSliceVar(&npdo.SystemLogMonitorConfigPaths, "system-log-monitors", - []string{}, "List of paths to system log monitor config files, comma separated.") - fs.MarkDeprecated("system-log-monitors", "replaced by --config.system-log-monitor. NPD will panic if both --system-log-monitors and --config.system-log-monitor are set.") - fs.StringSliceVar(&npdo.CustomPluginMonitorConfigPaths, "custom-plugin-monitors", - []string{}, "List of paths to custom plugin monitor config files, comma separated.") - fs.MarkDeprecated("custom-plugin-monitors", "replaced by --config.custom-plugin-monitor. NPD will panic if both --custom-plugin-monitors and --config.custom-plugin-monitor are set.") fs.BoolVar(&npdo.EnableK8sExporter, "enable-k8s-exporter", true, "Enables reporting to Kubernetes API server.") fs.StringVar(&npdo.ApiServerOverride, "apiserver-override", "", "Custom URI used to connect to Kubernetes ApiServer. This is ignored if --enable-k8s-exporter is false.") @@ -124,14 +93,10 @@ func (npdo *NodeProblemDetectorOptions) AddFlags(fs *pflag.FlagSet) { exporterHandler := exporters.GetExporterHandlerOrDie(exporterName) exporterHandler.Options.SetFlags(fs) } + for _, problemDaemonName := range problemdaemon.GetProblemDaemonNames() { - fs.StringSliceVar( - npdo.MonitorConfigPaths[problemDaemonName], - "config."+string(problemDaemonName), - []string{}, - fmt.Sprintf("Comma separated configurations for %v monitor. %v", - problemDaemonName, - problemdaemon.GetProblemDaemonHandlerOrDie(problemDaemonName).CmdOptionDescription)) + problemDaemonHandler := problemdaemon.GetProblemDaemonHandlerOrDie(problemDaemonName) + problemDaemonHandler.Options.SetFlags(fs) } } @@ -141,68 +106,6 @@ func (npdo *NodeProblemDetectorOptions) ValidOrDie() { panic(fmt.Sprintf("apiserver-override %q is not a valid HTTP URI: %v", npdo.ApiServerOverride, err)) } - - if len(npdo.SystemLogMonitorConfigPaths) != 0 { - panic("SystemLogMonitorConfigPaths is deprecated. It should have been reassigned to MonitorConfigPaths. This should not happen.") - } - if len(npdo.CustomPluginMonitorConfigPaths) != 0 { - panic("CustomPluginMonitorConfigPaths is deprecated. It should have been reassigned to MonitorConfigPaths. This should not happen.") - } - - configCount := 0 - for _, problemDaemonConfigPaths := range npdo.MonitorConfigPaths { - configCount += len(*problemDaemonConfigPaths) - } - if configCount == 0 { - panic("No configuration option for any problem daemon is specified.") - } -} - -// Plugin names for custom plugin monitor and system log monitor. -// Hard code them here to: -// 1) Handle deprecated flags for --system-log-monitors and --custom-plugin-monitors. -// 2) Avoid direct dependencies to packages in those plugins, so that those plugins -// can be disabled at compile time. -const ( - customPluginMonitorName = "custom-plugin-monitor" - systemLogMonitorName = "system-log-monitor" -) - -// SetConfigFromDeprecatedOptionsOrDie sets NPD option using deprecated options. -func (npdo *NodeProblemDetectorOptions) SetConfigFromDeprecatedOptionsOrDie() { - if len(npdo.SystemLogMonitorConfigPaths) != 0 { - if npdo.MonitorConfigPaths[systemLogMonitorName] == nil { - // As long as the problem daemon is registered, MonitorConfigPaths should - // not be nil. - panic("System log monitor is not supported") - } - - if len(*npdo.MonitorConfigPaths[systemLogMonitorName]) != 0 { - panic("Option --system-log-monitors is deprecated in favor of --config.system-log-monitor. They cannot be set at the same time.") - } - - *npdo.MonitorConfigPaths[systemLogMonitorName] = append( - *npdo.MonitorConfigPaths[systemLogMonitorName], - npdo.SystemLogMonitorConfigPaths...) - npdo.SystemLogMonitorConfigPaths = []string{} - } - - if len(npdo.CustomPluginMonitorConfigPaths) != 0 { - if npdo.MonitorConfigPaths[customPluginMonitorName] == nil { - // As long as the problem daemon is registered, MonitorConfigPaths should - // not be nil. - panic("Custom plugin monitor is not supported") - } - - if len(*npdo.MonitorConfigPaths[customPluginMonitorName]) != 0 { - panic("Option --custom-plugin-monitors is deprecated in favor of --config.custom-plugin-monitor. They cannot be set at the same time.") - } - - *npdo.MonitorConfigPaths[customPluginMonitorName] = append( - *npdo.MonitorConfigPaths[customPluginMonitorName], - npdo.CustomPluginMonitorConfigPaths...) - npdo.CustomPluginMonitorConfigPaths = []string{} - } } // SetNodeNameOrDie sets `NodeName` field with valid value. diff --git a/cmd/options/options_test.go b/cmd/options/options_test.go index 05902db7e..10b6a1b43 100644 --- a/cmd/options/options_test.go +++ b/cmd/options/options_test.go @@ -18,45 +18,11 @@ package options import ( "os" - "reflect" "testing" "github.com/stretchr/testify/assert" - - "k8s.io/node-problem-detector/pkg/types" ) -func equalMonitorConfigPaths(npdoX NodeProblemDetectorOptions, npdoY NodeProblemDetectorOptions) bool { - monitorConfigPathsX, monitorConfigPathsY := npdoX.MonitorConfigPaths, npdoY.MonitorConfigPaths - - if monitorConfigPathsX == nil && monitorConfigPathsY == nil { - return true - } - if monitorConfigPathsX == nil || monitorConfigPathsY == nil { - return false - } - if len(monitorConfigPathsX) != len(monitorConfigPathsY) { - return false - } - - for problemDaemonType, configPathsX := range monitorConfigPathsX { - configPathsY, ok := monitorConfigPathsY[problemDaemonType] - if !ok { - return false - } - if configPathsX == nil && configPathsY == nil { - continue - } - if configPathsX == nil || configPathsY == nil { - return false - } - if !reflect.DeepEqual(*configPathsX, *configPathsY) { - return false - } - } - return true -} - type options struct { Nodename string HostnameOverride string @@ -109,7 +75,7 @@ func TestSetNodeNameOrDie(t *testing.T) { } } - npdOpts := NewNodeProblemDetectorOptions() + npdOpts := &NodeProblemDetectorOptions{} npdOpts.HostnameOverride = ut.Meta.HostnameOverride npdOpts.SetNodeNameOrDie() @@ -120,112 +86,37 @@ func TestSetNodeNameOrDie(t *testing.T) { } func TestValidOrDie(t *testing.T) { - fooMonitorConfigMap := types.ProblemDaemonConfigPathMap{} - fooMonitorConfigMap["foo-monitor"] = &[]string{"config-a", "config-b"} - - emptyMonitorConfigMap := types.ProblemDaemonConfigPathMap{} - testCases := []struct { name string npdo NodeProblemDetectorOptions expectPanic bool }{ { - name: "default k8s exporter config", - npdo: NodeProblemDetectorOptions{ - MonitorConfigPaths: fooMonitorConfigMap, - }, + name: "default k8s exporter config", + npdo: NodeProblemDetectorOptions{}, expectPanic: false, }, { name: "enables k8s exporter config", npdo: NodeProblemDetectorOptions{ - ApiServerOverride: "", - EnableK8sExporter: true, - MonitorConfigPaths: fooMonitorConfigMap, + ApiServerOverride: "", + EnableK8sExporter: true, }, expectPanic: false, }, { name: "k8s exporter config with valid ApiServerOverride", npdo: NodeProblemDetectorOptions{ - ApiServerOverride: "127.0.0.1", - EnableK8sExporter: true, - MonitorConfigPaths: fooMonitorConfigMap, + ApiServerOverride: "127.0.0.1", + EnableK8sExporter: true, }, expectPanic: false, }, { name: "k8s exporter config with invalid ApiServerOverride", npdo: NodeProblemDetectorOptions{ - ApiServerOverride: ":foo", - EnableK8sExporter: true, - MonitorConfigPaths: fooMonitorConfigMap, - }, - expectPanic: true, - }, - { - name: "non-empty MonitorConfigPaths", - npdo: NodeProblemDetectorOptions{ - MonitorConfigPaths: fooMonitorConfigMap, - }, - expectPanic: false, - }, - { - name: "empty MonitorConfigPaths", - npdo: NodeProblemDetectorOptions{ - MonitorConfigPaths: emptyMonitorConfigMap, - }, - expectPanic: true, - }, - { - name: "un-initialized MonitorConfigPaths", - npdo: NodeProblemDetectorOptions{}, - expectPanic: true, - }, - { - name: "mixture of deprecated SystemLogMonitorConfigPaths and new MonitorConfigPaths", - npdo: NodeProblemDetectorOptions{ - SystemLogMonitorConfigPaths: []string{"config-a"}, - MonitorConfigPaths: fooMonitorConfigMap, - }, - expectPanic: true, - }, - { - name: "mixture of deprecated CustomPluginMonitorConfigPaths and new MonitorConfigPaths", - npdo: NodeProblemDetectorOptions{ - CustomPluginMonitorConfigPaths: []string{"config-a"}, - MonitorConfigPaths: fooMonitorConfigMap, - }, - expectPanic: true, - }, - { - name: "deprecated SystemLogMonitor option with empty MonitorConfigPaths", - npdo: NodeProblemDetectorOptions{ - SystemLogMonitorConfigPaths: []string{"config-a"}, - MonitorConfigPaths: emptyMonitorConfigMap, - }, - expectPanic: true, - }, - { - name: "deprecated SystemLogMonitor option with un-initialized MonitorConfigPaths", - npdo: NodeProblemDetectorOptions{ - SystemLogMonitorConfigPaths: []string{"config-a"}, - }, - expectPanic: true, - }, - { - name: "deprecated CustomPluginMonitor option with empty MonitorConfigPaths", - npdo: NodeProblemDetectorOptions{ - CustomPluginMonitorConfigPaths: []string{"config-b"}, - MonitorConfigPaths: emptyMonitorConfigMap, - }, - expectPanic: true, - }, - { - name: "deprecated CustomPluginMonitor option with un-initialized MonitorConfigPaths", - npdo: NodeProblemDetectorOptions{ - CustomPluginMonitorConfigPaths: []string{"config-b"}, + ApiServerOverride: ":foo", + EnableK8sExporter: true, }, expectPanic: true, }, @@ -241,142 +132,3 @@ func TestValidOrDie(t *testing.T) { }) } } - -func TestSetConfigFromDeprecatedOptionsOrDie(t *testing.T) { - testCases := []struct { - name string - orig NodeProblemDetectorOptions - wanted NodeProblemDetectorOptions - expectPanic bool - }{ - { - name: "no deprecated options", - orig: NodeProblemDetectorOptions{ - MonitorConfigPaths: types.ProblemDaemonConfigPathMap{ - systemLogMonitorName: &[]string{"config-a", "config-b"}, - customPluginMonitorName: &[]string{"config-c", "config-d"}, - }, - }, - expectPanic: false, - wanted: NodeProblemDetectorOptions{ - MonitorConfigPaths: types.ProblemDaemonConfigPathMap{ - systemLogMonitorName: &[]string{"config-a", "config-b"}, - customPluginMonitorName: &[]string{"config-c", "config-d"}, - }, - }, - }, - { - name: "correctly using deprecated options", - orig: NodeProblemDetectorOptions{ - SystemLogMonitorConfigPaths: []string{"config-a", "config-b"}, - CustomPluginMonitorConfigPaths: []string{"config-c", "config-d"}, - MonitorConfigPaths: types.ProblemDaemonConfigPathMap{ - customPluginMonitorName: &[]string{}, - systemLogMonitorName: &[]string{}, - }, - }, - expectPanic: false, - wanted: NodeProblemDetectorOptions{ - MonitorConfigPaths: types.ProblemDaemonConfigPathMap{ - systemLogMonitorName: &[]string{"config-a", "config-b"}, - customPluginMonitorName: &[]string{"config-c", "config-d"}, - }, - }, - }, - { - name: "using deprecated SystemLogMonitor option and new CustomPluginMonitor option", - orig: NodeProblemDetectorOptions{ - SystemLogMonitorConfigPaths: []string{"config-a", "config-b"}, - MonitorConfigPaths: types.ProblemDaemonConfigPathMap{ - customPluginMonitorName: &[]string{"config-c", "config-d"}, - systemLogMonitorName: &[]string{}, - }, - }, - expectPanic: false, - wanted: NodeProblemDetectorOptions{ - MonitorConfigPaths: types.ProblemDaemonConfigPathMap{ - systemLogMonitorName: &[]string{"config-a", "config-b"}, - customPluginMonitorName: &[]string{"config-c", "config-d"}, - }, - }, - }, - { - name: "using deprecated CustomPluginMonitor option and new SystemLogMonitor option", - orig: NodeProblemDetectorOptions{ - CustomPluginMonitorConfigPaths: []string{"config-a", "config-b"}, - MonitorConfigPaths: types.ProblemDaemonConfigPathMap{ - customPluginMonitorName: &[]string{}, - systemLogMonitorName: &[]string{"config-c", "config-d"}, - }, - }, - expectPanic: false, - wanted: NodeProblemDetectorOptions{ - MonitorConfigPaths: types.ProblemDaemonConfigPathMap{ - systemLogMonitorName: &[]string{"config-c", "config-d"}, - customPluginMonitorName: &[]string{"config-a", "config-b"}, - }, - }, - }, - { - name: "using deprecated & new options on SystemLogMonitor", - orig: NodeProblemDetectorOptions{ - SystemLogMonitorConfigPaths: []string{"config-a"}, - MonitorConfigPaths: types.ProblemDaemonConfigPathMap{ - systemLogMonitorName: &[]string{"config-b"}, - }, - }, - expectPanic: true, - }, - { - name: "using deprecated & new options on CustomPluginMonitor", - orig: NodeProblemDetectorOptions{ - CustomPluginMonitorConfigPaths: []string{"config-a"}, - MonitorConfigPaths: types.ProblemDaemonConfigPathMap{ - customPluginMonitorName: &[]string{"config-b"}, - }, - }, - expectPanic: true, - }, - { - name: "using deprecated options when SystemLogMonitor is not registered", - orig: NodeProblemDetectorOptions{ - SystemLogMonitorConfigPaths: []string{"config-a"}, - CustomPluginMonitorConfigPaths: []string{"config-b"}, - MonitorConfigPaths: types.ProblemDaemonConfigPathMap{ - customPluginMonitorName: &[]string{}, - }, - }, - expectPanic: true, - }, - { - name: "using deprecated options when CustomPluginMonitor is not registered", - orig: NodeProblemDetectorOptions{ - SystemLogMonitorConfigPaths: []string{"config-a"}, - CustomPluginMonitorConfigPaths: []string{"config-b"}, - MonitorConfigPaths: types.ProblemDaemonConfigPathMap{ - systemLogMonitorName: &[]string{}, - }, - }, - expectPanic: true, - }, - } - - for _, test := range testCases { - t.Run(test.name, func(t *testing.T) { - if test.expectPanic { - assert.Panics(t, test.orig.SetConfigFromDeprecatedOptionsOrDie, - "NPD option %+v is illegal. Expected SetConfigFromDeprecatedOptionsOrDie to panic.", test.orig) - } else { - assert.NotPanics(t, test.orig.SetConfigFromDeprecatedOptionsOrDie, - "NPD option %+v is illegal. Expected SetConfigFromDeprecatedOptionsOrDie to not panic.", test.orig) - if !equalMonitorConfigPaths(test.orig, test.wanted) { - t.Errorf("Expect to get NPD option %+v, but got %+v", test.wanted, test.orig) - } - assert.Len(t, test.orig.SystemLogMonitorConfigPaths, 0, - "SystemLogMonitorConfigPaths is deprecated and should to be cleared.") - assert.Len(t, test.orig.CustomPluginMonitorConfigPaths, 0, - "CustomPluginMonitorConfigPaths is deprecated and should to be cleared.") - } - }) - } -} diff --git a/pkg/custompluginmonitor/custom_plugin_monitor.go b/pkg/custompluginmonitor/custom_plugin_monitor.go index bc4a273e0..657df6405 100644 --- a/pkg/custompluginmonitor/custom_plugin_monitor.go +++ b/pkg/custompluginmonitor/custom_plugin_monitor.go @@ -19,6 +19,7 @@ package custompluginmonitor import ( "encoding/json" "io/ioutil" + "reflect" "time" "github.com/golang/glog" @@ -35,11 +36,30 @@ import ( const CustomPluginMonitorName = "custom-plugin-monitor" func init() { + clo := cpmtypes.CommandLineOptions{} problemdaemon.Register( CustomPluginMonitorName, types.ProblemDaemonHandler{ - CreateProblemDaemonOrDie: NewCustomPluginMonitorOrDie, - CmdOptionDescription: "Set to config file paths."}) + CreateProblemDaemonOrDie: NewCustomPluginMonitorsOrDie, + Options: &clo}) +} + +func NewCustomPluginMonitorsOrDie(clo types.CommandLineOptions) []types.Monitor { + cpmOptions, ok := clo.(*cpmtypes.CommandLineOptions) + if !ok { + glog.Fatalf("Wrong type for the command line options of Custom Plugin Monitors: %s.", reflect.TypeOf(clo)) + } + + if len(cpmOptions.CustomPluginMonitorConfigPaths) != 0 && len(cpmOptions.DeprecatedCustomPluginMonitorConfigPaths) != 0 { + glog.Fatalf("Option --custom-plugin-monitors is deprecated in favor of --config.custom-plugin-monitor. They cannot be set at the same time.") + } + cpmOptions.CustomPluginMonitorConfigPaths = append(cpmOptions.CustomPluginMonitorConfigPaths, cpmOptions.DeprecatedCustomPluginMonitorConfigPaths...) + + var monitors []types.Monitor + for _, configPath := range cpmOptions.CustomPluginMonitorConfigPaths { + monitors = append(monitors, NewCustomPluginMonitorOrDie(configPath)) + } + return monitors } type customPluginMonitor struct { diff --git a/pkg/custompluginmonitor/types/options.go b/pkg/custompluginmonitor/types/options.go new file mode 100644 index 000000000..d714be0ec --- /dev/null +++ b/pkg/custompluginmonitor/types/options.go @@ -0,0 +1,40 @@ +/* +Copyright 2019 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package types + +import ( + "github.com/spf13/pflag" +) + +type CommandLineOptions struct { + // DeprecatedCustomPluginMonitorConfigPaths specifies the list of paths to custom plugin monitor configuration + // files. DeprecatedCustomPluginMonitorConfigPaths is used by the deprecated option --custom-plugin-monitors. + DeprecatedCustomPluginMonitorConfigPaths []string + // CustomPluginMonitorConfigPaths specifies the list of paths to custom plugin monitor configuration + // files. CustomPluginMonitorConfigPaths is used by the option --config.custom-plugin-monitors. + CustomPluginMonitorConfigPaths []string +} + +func (clo *CommandLineOptions) SetFlags(fs *pflag.FlagSet) { + fs.StringSliceVar(&clo.DeprecatedCustomPluginMonitorConfigPaths, "custom-plugin-monitors", + []string{}, "List of paths to custom plugin monitor config files, comma separated.") + fs.MarkDeprecated("custom-plugin-monitors", + "replaced by --config.custom-plugin-monitor. NPD will panic if both --custom-plugin-monitors and --config.custom-plugin-monitor are set.") + + fs.StringSliceVar(&clo.CustomPluginMonitorConfigPaths, "config.custom-plugin-monitor", + []string{}, "List of paths to custom plugin monitor config files, comma separated.") +} diff --git a/pkg/problemdaemon/problem_daemon.go b/pkg/problemdaemon/problem_daemon.go index 79a1124a1..227d29ea5 100644 --- a/pkg/problemdaemon/problem_daemon.go +++ b/pkg/problemdaemon/problem_daemon.go @@ -19,8 +19,6 @@ package problemdaemon import ( "fmt" - "github.com/golang/glog" - "k8s.io/node-problem-detector/pkg/types" ) @@ -52,22 +50,11 @@ func GetProblemDaemonHandlerOrDie(problemDaemonType types.ProblemDaemonType) typ } // NewProblemDaemons creates all problem daemons based on the configurations provided. -func NewProblemDaemons(monitorConfigPaths types.ProblemDaemonConfigPathMap) []types.Monitor { - problemDaemonMap := make(map[string]types.Monitor) - for problemDaemonType, configs := range monitorConfigPaths { - for _, config := range *configs { - if _, ok := problemDaemonMap[config]; ok { - // Skip the config if it's duplicated. - glog.Warningf("Duplicated problem daemon configuration %q", config) - continue - } - problemDaemonMap[config] = handlers[problemDaemonType].CreateProblemDaemonOrDie(config) - } - } +func NewProblemDaemons() []types.Monitor { + var problemDaemons []types.Monitor - problemDaemons := []types.Monitor{} - for _, problemDaemon := range problemDaemonMap { - problemDaemons = append(problemDaemons, problemDaemon) + for _, handler := range handlers { + problemDaemons = append(problemDaemons, handler.CreateProblemDaemonOrDie(handler.Options)...) } return problemDaemons } diff --git a/pkg/problemdaemon/problem_daemon_test.go b/pkg/problemdaemon/problem_daemon_test.go index 2911b69d8..e161eabd0 100644 --- a/pkg/problemdaemon/problem_daemon_test.go +++ b/pkg/problemdaemon/problem_daemon_test.go @@ -25,20 +25,20 @@ import ( ) func TestRegistration(t *testing.T) { - fooMonitorFactory := func(configPath string) types.Monitor { - return nil + fooMonitorFactory := func(types.CommandLineOptions) []types.Monitor { + return []types.Monitor{} } fooMonitorHandler := types.ProblemDaemonHandler{ CreateProblemDaemonOrDie: fooMonitorFactory, - CmdOptionDescription: "foo option", + Options: nil, } - barMonitorFactory := func(configPath string) types.Monitor { - return nil + barMonitorFactory := func(types.CommandLineOptions) []types.Monitor { + return []types.Monitor{} } barMonitorHandler := types.ProblemDaemonHandler{ CreateProblemDaemonOrDie: barMonitorFactory, - CmdOptionDescription: "bar option", + Options: nil, } Register("foo", fooMonitorHandler) @@ -46,21 +46,18 @@ func TestRegistration(t *testing.T) { expectedProblemDaemonNames := []types.ProblemDaemonType{"foo", "bar"} problemDaemonNames := GetProblemDaemonNames() - assert.ElementsMatch(t, expectedProblemDaemonNames, problemDaemonNames) - assert.Equal(t, "foo option", GetProblemDaemonHandlerOrDie("foo").CmdOptionDescription) - assert.Equal(t, "bar option", GetProblemDaemonHandlerOrDie("bar").CmdOptionDescription) handlers = make(map[types.ProblemDaemonType]types.ProblemDaemonHandler) } func TestGetProblemDaemonHandlerOrDie(t *testing.T) { - fooMonitorFactory := func(configPath string) types.Monitor { - return nil + fooMonitorFactory := func(types.CommandLineOptions) []types.Monitor { + return []types.Monitor{} } fooMonitorHandler := types.ProblemDaemonHandler{ CreateProblemDaemonOrDie: fooMonitorFactory, - CmdOptionDescription: "foo option", + Options: nil, } Register("foo", fooMonitorHandler) diff --git a/pkg/systemlogmonitor/log_monitor.go b/pkg/systemlogmonitor/log_monitor.go index 777f72682..7698a74b8 100644 --- a/pkg/systemlogmonitor/log_monitor.go +++ b/pkg/systemlogmonitor/log_monitor.go @@ -19,6 +19,7 @@ package systemlogmonitor import ( "encoding/json" "io/ioutil" + "reflect" "time" "github.com/golang/glog" @@ -37,11 +38,30 @@ import ( const SystemLogMonitorName = "system-log-monitor" func init() { + clo := commandLineOptions{} problemdaemon.Register( SystemLogMonitorName, types.ProblemDaemonHandler{ - CreateProblemDaemonOrDie: NewLogMonitorOrDie, - CmdOptionDescription: "Set to config file paths."}) + CreateProblemDaemonOrDie: NewLogMonitorsOrDie, + Options: &clo}) +} + +func NewLogMonitorsOrDie(clo types.CommandLineOptions) []types.Monitor { + lmOptions, ok := clo.(*commandLineOptions) + if !ok { + glog.Fatalf("Wrong type for the command line options of Log Monitors: %s.", reflect.TypeOf(clo)) + } + + if len(lmOptions.SystemLogMonitorConfigPaths) != 0 && len(lmOptions.DeprecatedSystemLogMonitorConfigPaths) != 0 { + glog.Fatalf("Option --system-log-monitors is deprecated in favor of --config.system-log-monitor. They cannot be set at the same time.") + } + lmOptions.SystemLogMonitorConfigPaths = append(lmOptions.SystemLogMonitorConfigPaths, lmOptions.DeprecatedSystemLogMonitorConfigPaths...) + + var monitors []types.Monitor + for _, configPath := range lmOptions.SystemLogMonitorConfigPaths { + monitors = append(monitors, NewLogMonitorOrDie(configPath)) + } + return monitors } type logMonitor struct { diff --git a/pkg/systemlogmonitor/options.go b/pkg/systemlogmonitor/options.go new file mode 100644 index 000000000..982cd90ea --- /dev/null +++ b/pkg/systemlogmonitor/options.go @@ -0,0 +1,39 @@ +/* +Copyright 2019 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package systemlogmonitor + +import ( + "github.com/spf13/pflag" +) + +type commandLineOptions struct { + // DeprecatedSystemLogMonitorConfigPaths specifies the list of paths to system log monitor configuration + // files. DeprecatedSystemLogMonitorConfigPaths is used by the deprecated option --system-log-monitors. + DeprecatedSystemLogMonitorConfigPaths []string + // SystemLogMonitorConfigPaths specifies the list of paths to system log monitor configuration + // files. SystemLogMonitorConfigPaths is used by the option --config.system-log-monitors. + SystemLogMonitorConfigPaths []string +} + +func (clo *commandLineOptions) SetFlags(fs *pflag.FlagSet) { + fs.StringSliceVar(&clo.DeprecatedSystemLogMonitorConfigPaths, "system-log-monitors", + []string{}, "List of paths to system log monitor config files, comma separated.") + fs.MarkDeprecated("system-log-monitors", "replaced by --config.system-log-monitor. NPD will panic if both --system-log-monitors and --config.system-log-monitor are set.") + + fs.StringSliceVar(&clo.SystemLogMonitorConfigPaths, "config.system-log-monitor", + []string{}, "List of paths to system log monitor config files, comma separated.") +} diff --git a/pkg/systemstatsmonitor/system_stats_monitor.go b/pkg/systemstatsmonitor/system_stats_monitor.go index 0e7fb367b..789a7be0f 100644 --- a/pkg/systemstatsmonitor/system_stats_monitor.go +++ b/pkg/systemstatsmonitor/system_stats_monitor.go @@ -19,6 +19,7 @@ package systemstatsmonitor import ( "encoding/json" "io/ioutil" + "reflect" "time" "github.com/golang/glog" @@ -32,9 +33,23 @@ import ( const SystemStatsMonitorName = "system-stats-monitor" func init() { + clo := ssmtypes.CommandLineOptions{} problemdaemon.Register(SystemStatsMonitorName, types.ProblemDaemonHandler{ - CreateProblemDaemonOrDie: NewSystemStatsMonitorOrDie, - CmdOptionDescription: "Set to config file paths."}) + CreateProblemDaemonOrDie: NewSystemStatsMonitorsOrDie, + Options: &clo}) +} + +func NewSystemStatsMonitorsOrDie(clo types.CommandLineOptions) []types.Monitor { + ssmOptions, ok := clo.(*ssmtypes.CommandLineOptions) + if !ok { + glog.Fatalf("Wrong type for the command line options of System Stats Monitors: %s.", reflect.TypeOf(clo)) + } + + var monitors []types.Monitor + for _, configPath := range ssmOptions.SystemStatsMonitorConfigPaths { + monitors = append(monitors, NewSystemStatsMonitorOrDie(configPath)) + } + return monitors } type systemStatsMonitor struct { diff --git a/pkg/systemstatsmonitor/types/options.go b/pkg/systemstatsmonitor/types/options.go new file mode 100644 index 000000000..b7c2286d5 --- /dev/null +++ b/pkg/systemstatsmonitor/types/options.go @@ -0,0 +1,32 @@ +/* +Copyright 2019 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package types + +import ( + "github.com/spf13/pflag" +) + +type CommandLineOptions struct { + // SystemStatsMonitorConfigPaths specifies the list of paths to system stats monitor configuration + // files. SystemStatsMonitorConfigPaths is used by the option --config.system-stats-monitors. + SystemStatsMonitorConfigPaths []string +} + +func (clo *CommandLineOptions) SetFlags(fs *pflag.FlagSet) { + fs.StringSliceVar(&clo.SystemStatsMonitorConfigPaths, "config.system-stats-monitor", + []string{}, "List of paths to system stats monitor config files, comma separated.") +} diff --git a/pkg/types/types.go b/pkg/types/types.go index 1a36804d6..802249d6b 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -121,17 +121,12 @@ type Exporter interface { // One type of problem daemon may be used to initialize multiple problem daemon instances. type ProblemDaemonType string -// ProblemDaemonConfigPathMap represents configurations on all types of problem daemons: -// 1) Each key represents a type of problem daemon. -// 2) Each value represents the config file paths to that type of problem daemon. -type ProblemDaemonConfigPathMap map[ProblemDaemonType]*[]string - // ProblemDaemonHandler represents the initialization handler for a type problem daemon. type ProblemDaemonHandler struct { - // CreateProblemDaemonOrDie initializes a problem daemon, panic if error occurs. - CreateProblemDaemonOrDie func(string) Monitor - // CmdOptionDescription explains how to configure the problem daemon from command line arguments. - CmdOptionDescription string + // CreateProblemDaemonOrDie initializes problem daemons from command line options, panic if error occurs. + CreateProblemDaemonOrDie func(CommandLineOptions) []Monitor + // Options specifies the command line options for this problem daemon. + Options CommandLineOptions } // ExporterType is the type of the exporter. @@ -141,7 +136,7 @@ type ExporterType string type ExporterHandler struct { // CreateExporterOrDie initializes an exporter, panic if error occurs. CreateExporterOrDie func(CommandLineOptions) Exporter - // CmdOptionDescription explains how to configure the exporter from command line arguments. + // Options specifies the command line options for this exporter. Options CommandLineOptions }