From 2319906d9728318bf65483ccc2f67d26499cc89f Mon Sep 17 00:00:00 2001 From: Jonathon Anderson Date: Mon, 10 Apr 2023 19:26:06 -0600 Subject: [PATCH 1/2] Fix writing of nodes.conf The caching functionality of nodes.conf, and the optional persistence functionality that came with it, appears to have been preventing actually writing the configuration to disk in all cases. This change fixes the ability to write to nodes.conf. Somewhat opinionatedly, this change removes the caching functionality of nodes.conf. The tests have also been updated to use real files for i/o rather than tying into the caching system, making them more accurately test the ability of Warewulf to actually read and write its configuration. Fixes #779 Signed-off-by: Jonathon Anderson --- .../app/wwctl/container/list/main_test.go | 2 +- internal/app/wwctl/node/add/main.go | 2 +- internal/app/wwctl/node/add/main_test.go | 37 ++++++++----- internal/app/wwctl/node/list/main_test.go | 26 ++++++--- internal/app/wwctl/profile/list/main_test.go | 25 ++++++--- internal/pkg/node/constructors.go | 54 +++++-------------- internal/pkg/node/datastructure.go | 2 - internal/pkg/node/modifiers.go | 43 ++++++++------- 8 files changed, 96 insertions(+), 95 deletions(-) diff --git a/internal/app/wwctl/container/list/main_test.go b/internal/app/wwctl/container/list/main_test.go index 8a6b4b1d..f87702e9 100644 --- a/internal/app/wwctl/container/list/main_test.go +++ b/internal/app/wwctl/container/list/main_test.go @@ -60,7 +60,7 @@ WW_INTERNAL: 0 assert.NoError(t, err) warewulfd.SetNoDaemon() for _, tt := range tests { - _, err = node.TestNew([]byte(tt.inDb)) + _, err = node.Parse([]byte(tt.inDb)) assert.NoError(t, err) t.Logf("Running test: %s\n", tt.name) t.Run(tt.name, func(t *testing.T) { diff --git a/internal/app/wwctl/node/add/main.go b/internal/app/wwctl/node/add/main.go index 94888e59..694e3e13 100644 --- a/internal/app/wwctl/node/add/main.go +++ b/internal/app/wwctl/node/add/main.go @@ -35,7 +35,7 @@ func CobraRunE(vars *variables) func(cmd *cobra.Command, args []string) error { } buffer, err := yaml.Marshal(vars.nodeConf) if err != nil { - wwlog.Error("Cant marshall nodeInfo", err) + wwlog.Error("Can't marshall nodeInfo", err) return err } set := wwapiv1.NodeAddParameter{ diff --git a/internal/app/wwctl/node/add/main_test.go b/internal/app/wwctl/node/add/main_test.go index 8fa8ed35..0781d6b9 100644 --- a/internal/app/wwctl/node/add/main_test.go +++ b/internal/app/wwctl/node/add/main_test.go @@ -2,6 +2,7 @@ package add import ( "bytes" + "os" "testing" "github.com/hpcng/warewulf/internal/pkg/node" @@ -235,21 +236,28 @@ nodes: ipaddr: 10.10.0.3 `}, } - conf_yml := ` -WW_INTERNAL: 0 - ` - nodes_yml := ` -WW_INTERNAL: 43 -` - conf := warewulfconf.New() - err := conf.Read([]byte(conf_yml)) - assert.NoError(t, err) - db, err := node.TestNew([]byte(nodes_yml)) - assert.NoError(t, err) + conf_yml := `WW_INTERNAL: 0` + tempWarewulfConf, warewulfConfErr := os.CreateTemp("", "warewulf.conf-") + assert.NoError(t, warewulfConfErr) + defer os.Remove(tempWarewulfConf.Name()) + _, warewulfConfErr = tempWarewulfConf.Write([]byte(conf_yml)) + assert.NoError(t, warewulfConfErr) + assert.NoError(t, tempWarewulfConf.Sync()) + warewulfconf.ConfigFile = tempWarewulfConf.Name() + + nodes_yml := `WW_INTERNAL: 43` + tempNodeConf, nodesConfErr := os.CreateTemp("", "nodes.conf-") + assert.NoError(t, nodesConfErr) + defer os.Remove(tempNodeConf.Name()) + node.ConfigFile = tempNodeConf.Name() warewulfd.SetNoDaemon() for _, tt := range tests { - db, err = node.TestNew([]byte(nodes_yml)) + var err error + _, err = tempNodeConf.Seek(0, 0) assert.NoError(t, err) + assert.NoError(t, tempNodeConf.Truncate(0)) + _, err = tempNodeConf.Write([]byte(nodes_yml)) + assert.NoError(t, tempNodeConf.Sync()) t.Logf("Running test: %s\n", tt.name) t.Run(tt.name, func(t *testing.T) { baseCmd := GetCommand() @@ -262,7 +270,10 @@ WW_INTERNAL: 43 t.Errorf("Got unwanted error: %s", err) t.FailNow() } - dump := string(db.DBDump()) + config, configErr := node.New() + assert.NoError(t, configErr) + dumpBytes, _ := config.Dump() + dump := string(dumpBytes) if dump != tt.outDb { t.Errorf("DB dump is wrong, got:'%s'\nwant:'%s'", dump, tt.outDb) t.FailNow() diff --git a/internal/app/wwctl/node/list/main_test.go b/internal/app/wwctl/node/list/main_test.go index 52eadfb4..679595dd 100644 --- a/internal/app/wwctl/node/list/main_test.go +++ b/internal/app/wwctl/node/list/main_test.go @@ -35,17 +35,27 @@ nodes: - default `}, } - conf_yml := ` -WW_INTERNAL: 0 - ` - conf := warewulfconf.New() - err := conf.Read([]byte(conf_yml)) - assert.NoError(t, err) - assert.NoError(t, err) + conf_yml := `WW_INTERNAL: 0` + tempWarewulfConf, warewulfConfErr := os.CreateTemp("", "warewulf.conf-") + assert.NoError(t, warewulfConfErr) + defer os.Remove(tempWarewulfConf.Name()) + _, warewulfConfErr = tempWarewulfConf.Write([]byte(conf_yml)) + assert.NoError(t, warewulfConfErr) + assert.NoError(t, tempWarewulfConf.Sync()) + warewulfconf.ConfigFile = tempWarewulfConf.Name() + + tempNodeConf, nodesConfErr := os.CreateTemp("", "nodes.conf-") + assert.NoError(t, nodesConfErr) + defer os.Remove(tempNodeConf.Name()) + node.ConfigFile = tempNodeConf.Name() warewulfd.SetNoDaemon() for _, tt := range tests { - _, err = node.TestNew([]byte(tt.inDb)) + var err error + _, err = tempNodeConf.Seek(0, 0) assert.NoError(t, err) + assert.NoError(t, tempNodeConf.Truncate(0)) + _, err = tempNodeConf.Write([]byte(tt.inDb)) + assert.NoError(t, tempNodeConf.Sync()) t.Logf("Running test: %s\n", tt.name) t.Run(tt.name, func(t *testing.T) { baseCmd := GetCommand() diff --git a/internal/app/wwctl/profile/list/main_test.go b/internal/app/wwctl/profile/list/main_test.go index 4e61ef18..36ee36c0 100644 --- a/internal/app/wwctl/profile/list/main_test.go +++ b/internal/app/wwctl/profile/list/main_test.go @@ -36,16 +36,27 @@ nodes: }, } - conf_yml := ` -WW_INTERNAL: 0 - ` + conf_yml := `WW_INTERNAL: 0` + tempWarewulfConf, warewulfConfErr := os.CreateTemp("", "warewulf.conf-") + assert.NoError(t, warewulfConfErr) + defer os.Remove(tempWarewulfConf.Name()) + _, warewulfConfErr = tempWarewulfConf.Write([]byte(conf_yml)) + assert.NoError(t, warewulfConfErr) + assert.NoError(t, tempWarewulfConf.Sync()) + warewulfconf.ConfigFile = tempWarewulfConf.Name() - conf := warewulfconf.New() - err := conf.Read([]byte(conf_yml)) - assert.NoError(t, err) + tempNodeConf, nodesConfErr := os.CreateTemp("", "nodes.conf-") + assert.NoError(t, nodesConfErr) + defer os.Remove(tempNodeConf.Name()) + node.ConfigFile = tempNodeConf.Name() warewulfd.SetNoDaemon() for _, tt := range tests { - _, err = node.TestNew([]byte(tt.inDb)) + var err error + _, err = tempNodeConf.Seek(0, 0) + assert.NoError(t, err) + assert.NoError(t, tempNodeConf.Truncate(0)) + _, err = tempNodeConf.Write([]byte(tt.inDb)) + assert.NoError(t, tempNodeConf.Sync()) assert.NoError(t, err) t.Logf("Running test: %s\n", tt.name) t.Run(tt.name, func(t *testing.T) { diff --git a/internal/pkg/node/constructors.go b/internal/pkg/node/constructors.go index 578cb094..48f11d0c 100644 --- a/internal/pkg/node/constructors.go +++ b/internal/pkg/node/constructors.go @@ -17,8 +17,6 @@ import ( var ConfigFile string var DefaultConfig string -var cachedDB NodeYaml - // used as fallback if DefaultConfig can't be read var FallBackConf = `--- defaultnode: @@ -47,32 +45,33 @@ func init() { if DefaultConfig == "" { DefaultConfig = path.Join(conf.Paths.Datadir, "warewulf/defaults.conf") } - cachedDB.current = false - cachedDB.persist = true } /* Creates a new nodeDb object from the actual configuration */ func New() (NodeYaml, error) { - if cachedDB.current { - wwlog.Debug("Returning cached object") - return cachedDB, nil - } - var ret NodeYaml - wwlog.Verbose("Opening node configuration file: %s", ConfigFile) data, err := os.ReadFile(ConfigFile) if err != nil { - return ret, err + return NodeYaml{}, err } + return Parse(data) +} + +func Parse(data []byte) (NodeYaml, error) { + var ret NodeYaml + var err error wwlog.Debug("Unmarshaling the node configuration") err = yaml.Unmarshal(data, &ret) if err != nil { return ret, err } wwlog.Debug("Checking nodes for types") + if ret.Nodes == nil { + ret.Nodes = map[string]*NodeConf{} + } for nodeName, node := range ret.Nodes { err = node.Check() if err != nil { @@ -80,6 +79,9 @@ func New() (NodeYaml, error) { return ret, err } } + if ret.NodeProfiles == nil { + ret.NodeProfiles = map[string]*NodeConf{} + } for profileName, profile := range ret.NodeProfiles { err = profile.Check() if err != nil { @@ -89,39 +91,9 @@ func New() (NodeYaml, error) { } wwlog.Debug("Returning node object") - cachedDB = ret - cachedDB.current = true return ret, nil } -/* -Creates a database object from a given buffer, always create -a new object, never return the cached one. -*/ -func TestNew(buffer []byte) (db NodeYaml, err error) { - db.NodeProfiles = make(map[string]*NodeConf) - db.Nodes = make(map[string]*NodeConf) - err = yaml.Unmarshal(buffer, &db) - db.persist = false - cachedDB = db - cachedDB.current = true - wwlog.Debug("Created cached object") - return -} - -func (config *NodeYaml) DBDump() (buffer []byte) { - for _, n := range config.Nodes { - n.Flatten() - } - for _, p := range config.NodeProfiles { - p.Flatten() - } - buffer, err := yaml.Marshal(config) - if err != nil { - wwlog.Warn("porblems on dumping nodedb: %s", err) - } - return -} /* Get all the nodes of a configuration. This function also merges diff --git a/internal/pkg/node/datastructure.go b/internal/pkg/node/datastructure.go index 2c27c289..ab22d3c8 100644 --- a/internal/pkg/node/datastructure.go +++ b/internal/pkg/node/datastructure.go @@ -10,8 +10,6 @@ type NodeYaml struct { WWInternal int `yaml:"WW_INTERNAL"` NodeProfiles map[string]*NodeConf Nodes map[string]*NodeConf - current bool - persist bool } /* diff --git a/internal/pkg/node/modifiers.go b/internal/pkg/node/modifiers.go index 2c644657..8513b5ed 100644 --- a/internal/pkg/node/modifiers.go +++ b/internal/pkg/node/modifiers.go @@ -116,6 +116,26 @@ func (config *NodeYaml) ProfileUpdate(profile NodeInfo) error { Write the the NodeYaml to disk. */ func (config *NodeYaml) Persist() error { + out, dumpErr := config.Dump() + if dumpErr != nil { + wwlog.Error("%s", dumpErr) + os.Exit(1) + } + file, err := os.OpenFile(ConfigFile, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644) + if err != nil { + wwlog.Error("%s", err) + os.Exit(1) + } + defer file.Close() + _, err = file.WriteString(string(out)) + if err != nil { + return err + } + return nil +} + + +func (config *NodeYaml) Dump() ([]byte, error) { // flatten out profiles and nodes for _, val := range config.NodeProfiles { val.Flatten() @@ -123,26 +143,5 @@ func (config *NodeYaml) Persist() error { for _, val := range config.Nodes { val.Flatten() } - out, err := yaml.Marshal(config) - if err != nil { - return err - } - // Can't persist for unit tests - if !config.persist { - return err - } - file, err := os.OpenFile(ConfigFile, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644) - if err != nil { - wwlog.Error("%s", err) - os.Exit(1) - } - - defer file.Close() - - _, err = file.WriteString(string(out)) - if err != nil { - return err - } - - return nil + return yaml.Marshal(config) } From cc714008ef7fa1d92e6ec0c540c61d95ae05e979 Mon Sep 17 00:00:00 2001 From: Jonathon Anderson Date: Sun, 16 Apr 2023 07:24:14 -0600 Subject: [PATCH 2/2] Add doc comments for two methods Signed-off-by: Jonathon Anderson --- internal/pkg/node/constructors.go | 6 +++++- internal/pkg/node/modifiers.go | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/pkg/node/constructors.go b/internal/pkg/node/constructors.go index 48f11d0c..a2fb9d4a 100644 --- a/internal/pkg/node/constructors.go +++ b/internal/pkg/node/constructors.go @@ -48,7 +48,7 @@ func init() { } /* -Creates a new nodeDb object from the actual configuration +Creates a new nodeDb object from the on-disk configuration */ func New() (NodeYaml, error) { wwlog.Verbose("Opening node configuration file: %s", ConfigFile) @@ -60,6 +60,10 @@ func New() (NodeYaml, error) { } +// Parse constructs a new nodeDb object from an input YAML +// document. Passes any errors return from yaml.Unmarshal. Returns an +// error if any parsed value is not of a valid type for the given +// parameter. func Parse(data []byte) (NodeYaml, error) { var ret NodeYaml var err error diff --git a/internal/pkg/node/modifiers.go b/internal/pkg/node/modifiers.go index 8513b5ed..8f2081d7 100644 --- a/internal/pkg/node/modifiers.go +++ b/internal/pkg/node/modifiers.go @@ -135,6 +135,8 @@ func (config *NodeYaml) Persist() error { } +// Dump returns a YAML document representing the nodeDb +// instance. Passes through any errors generated by yaml.Marshal. func (config *NodeYaml) Dump() ([]byte, error) { // flatten out profiles and nodes for _, val := range config.NodeProfiles {