Skip to content

Commit

Permalink
feat(daemon): make config file path configurable (#357)
Browse files Browse the repository at this point in the history
* add config file as argument instead of doing with the default hardcoded filepath
* fix test cases for config options

Signed-off-by: Akhil Mohan <[email protected]>
  • Loading branch information
akhilerm authored and kmova committed Dec 27, 2019
1 parent 1c5a671 commit 768321e
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 24 deletions.
8 changes: 8 additions & 0 deletions cmd/ndm_daemonset/app/command/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package command

import (
goflag "flag"
"github.com/openebs/node-disk-manager/cmd/ndm_daemonset/controller"
"github.com/openebs/node-disk-manager/pkg/version"

"github.com/openebs/node-disk-manager/pkg/util"
Expand All @@ -26,8 +27,12 @@ import (
"k8s.io/klog"
)

// options is the options with which the daemon is to be run
var options controller.NDMOptions

// NewNodeDiskManager creates a new ndm.
func NewNodeDiskManager() (*cobra.Command, error) {

// Create a new command
cmd := &cobra.Command{
Use: "ndm",
Expand All @@ -38,6 +43,9 @@ func NewNodeDiskManager() (*cobra.Command, error) {
}

pflag.CommandLine.AddGoFlagSet(goflag.CommandLine)
cmd.PersistentFlags().StringVar(&options.ConfigFilePath, "config",
controller.DefaultConfigFilePath,
"Path to config file")
_ = goflag.CommandLine.Parse([]string{})

cmd.AddCommand(
Expand Down
3 changes: 2 additions & 1 deletion cmd/ndm_daemonset/app/command/device-list.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ func deviceList() error {
if err != nil {
return err
}
diskList, err := ctrl.ListDiskResource()
// TODO @akhilerm should pass the filter as args to List, so that all devices will be listed
diskList, err := ctrl.ListBlockDeviceResource()

This comment has been minimized.

Copy link
@cnbailian

cnbailian Oct 22, 2020

The disk on the node cannot be printed correctly now because of the lack of call to ctrl.SetControllerOptions.

This comment has been minimized.

Copy link
@akhilerm

akhilerm Oct 22, 2020

Author Contributor

@cnbailian Thanks for raising this issue. I have raised a PR to fix it . #504. PTAL.

if err != nil {
return err
}
Expand Down
7 changes: 7 additions & 0 deletions cmd/ndm_daemonset/app/command/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ func NewCmdStart() *cobra.Command {
fmt.Println(err)
os.Exit(1)
}

// set the NDM config from the options
err = ctrl.SetControllerOptions(options)
if err != nil {
fmt.Println(err)
os.Exit(1)
}
// Broadcast starts broadcasting controller pointer. Using this
// each probe and filter registers themselves.
ctrl.Broadcast()
Expand Down
30 changes: 20 additions & 10 deletions cmd/ndm_daemonset/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ var Namespace string
// Each probe can get the copy of controller struct any time they need to read the channel.
var ControllerBroadcastChannel = make(chan *Controller)

// NDMOptions defines the options to run the NDM daemon
type NDMOptions struct {
ConfigFilePath string
}

// Controller is the controller implementation for disk resources
type Controller struct {
config *rest.Config // config is the generated config using kubeconfig/incluster config
Expand Down Expand Up @@ -136,27 +141,32 @@ func NewController() (*Controller, error) {
return controller, err
}

controller.SetNDMConfig()
controller.Filters = make([]*Filter, 0)
controller.Probes = make([]*Probe, 0)
controller.NodeAttributes = make(map[string]string, 0)
controller.Mutex = &sync.Mutex{}

// get the namespace in which NDM is installed
Namespace, err = getNamespace()
if err != nil {
return controller, err
}

if err := controller.setNodeAttributes(); err != nil {
return nil, err
}

controller.WaitForDiskCRD()
controller.WaitForBlockDeviceCRD()
return controller, nil
}

// SetControllerOptions sets the various attributes and options
// on the controller
func (c *Controller) SetControllerOptions(opts NDMOptions) error {
// set the config for running NDM daemon
c.SetNDMConfig(opts)
c.Filters = make([]*Filter, 0)
c.Probes = make([]*Probe, 0)
c.NodeAttributes = make(map[string]string, 0)
c.Mutex = &sync.Mutex{}
if err := c.setNodeAttributes(); err != nil {
return err
}
return nil
}

// newClientSet set Clientset field in Controller struct
// if it gets Client from config. It returns the generated
// client, else it returns error
Expand Down
11 changes: 7 additions & 4 deletions cmd/ndm_daemonset/controller/ndmconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@ import (
"k8s.io/klog"
)

// ConfigFilePath contains configmap file path
var ConfigFilePath = "/host/node-disk-manager.config"
const (
// DefaultConfigFilePath is the default path at which config is present inside
// container
DefaultConfigFilePath = "/host/node-disk-manager.config"
)

// NodeDiskManagerConfig contains congigs of probes and filters
type NodeDiskManagerConfig struct {
Expand All @@ -51,8 +54,8 @@ type FilterConfig struct {

// SetNDMConfig sets config for probes and filters which user provides via configmap. If
// no configmap present then ndm will load default config for each probes and filters.
func (c *Controller) SetNDMConfig() {
data, err := ioutil.ReadFile(ConfigFilePath)
func (c *Controller) SetNDMConfig(opts NDMOptions) {
data, err := ioutil.ReadFile(opts.ConfigFilePath)
if err != nil {
c.NDMConfig = nil
klog.Error("unable to set ndm config : ", err)
Expand Down
22 changes: 13 additions & 9 deletions cmd/ndm_daemonset/controller/ndmconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,18 @@ import (

func TestSetNDMConfig(t *testing.T) {
fakeConfigFilePath := "/tmp/fakendm.config"
defaultConfigFilePath := ConfigFilePath

options := NDMOptions{
ConfigFilePath: fakeConfigFilePath,
}

// test case : when file not present
fakeController := &Controller{}
fakeController.SetNDMConfig()
fakeController.SetNDMConfig(options)
if fakeController.NDMConfig != nil {
t.Error("NDMConfig should nil for invalid file path")
}
// test case for invalid json
ConfigFilePath = fakeConfigFilePath

fileContent := []byte(`{
"probeconfigs": [
{
Expand Down Expand Up @@ -85,26 +87,28 @@ func TestSetNDMConfig(t *testing.T) {
}
expectedNDMConfig.FilterConfigs = append(expectedNDMConfig.FilterConfigs, expectedFilterConfig)
expectedNDMConfig.ProbeConfigs = append(expectedNDMConfig.ProbeConfigs, expectedProbeConfig)
ConfigFilePath = fakeConfigFilePath

err = ioutil.WriteFile(fakeConfigFilePath, fileContent, 0644)
if err != nil {
t.Fatal(err)
}
fakeController.SetNDMConfig()
fakeController.SetNDMConfig(options)
assert.Equal(t, expectedNDMConfig, *fakeController.NDMConfig)
os.Remove(fakeConfigFilePath)
ConfigFilePath = defaultConfigFilePath

}

func TestController_SetNDMConfig_yaml(t *testing.T) {
fakeConfigFilePath := "/tmp/fakendm-yaml.config"
writeTestYaml(t, fakeConfigFilePath)
defer os.Remove(fakeConfigFilePath)

ConfigFilePath = fakeConfigFilePath
options := NDMOptions{
ConfigFilePath: fakeConfigFilePath,
}

ctrl := &Controller{}
ctrl.SetNDMConfig()
ctrl.SetNDMConfig(options)

assert.NotNil(t, ctrl.NDMConfig)

Expand Down

0 comments on commit 768321e

Please sign in to comment.