wwctl node add without global variables

Signed-off-by: Christian Goll <cgoll@suse.de>

internal/app/wwctl/node/add/main.go

@@ -1,8 +1,6 @@
 package add
 
 import (
-	"os"
-
 	"gopkg.in/yaml.v2"
 
 	apinode "github.com/hpcng/warewulf/internal/pkg/api/node"
@@ -11,27 +9,33 @@ import (
 	"github.com/spf13/cobra"
 )
 
-func CobraRunE(cmd *cobra.Command, args []string) error {
+/*
-	// remove the default network as all network values are assigned
+RunE needs a function of type func(*cobraCommand,[]string) err, but
-	// to this network
+in order to avoid global variables which mess up testing a function of
-	if _, ok := NodeConf.NetDevs["default"]; ok && NetName != "" {
+the required type is returned
-		netDev := *NodeConf.NetDevs["default"]
+*/
-		NodeConf.NetDevs[NetName] = &netDev
+func CobraRunE(vars *variables) func(cmd *cobra.Command, args []string) error {
-		delete(NodeConf.NetDevs, "default")
+	return func(cmd *cobra.Command, args []string) error {
-	} else {
+		// remove the default network as all network values are assigned
-		if NodeConf.NetDevs["default"].Empty() {
+		// to this network
-			delete(NodeConf.NetDevs, "default")
+		if _, ok := vars.nodeConf.NetDevs["default"]; ok && vars.netName != "" {
+			netDev := *vars.nodeConf.NetDevs["default"]
+			vars.nodeConf.NetDevs[vars.netName] = &netDev
+			delete(vars.nodeConf.NetDevs, "default")
+		} else {
+			if vars.nodeConf.NetDevs["default"].Empty() {
+				delete(vars.nodeConf.NetDevs, "default")
+			}
 		}
+		buffer, err := yaml.Marshal(vars.nodeConf)
+		if err != nil {
+			wwlog.Error("Cant marshall nodeInfo", err)
+			return err
+		}
+		set := wwapiv1.NodeAddParameter{
+			NodeConfYaml: string(buffer[:]),
+			NodeNames:    args,
+		}
+		return apinode.NodeAdd(&set)
 	}
-	buffer, err := yaml.Marshal(NodeConf)
-	if err != nil {
-		wwlog.Error("Cant marshall nodeInfo", err)
-		os.Exit(1)
-	}
-	set := wwapiv1.NodeAddParameter{
-		NodeConfYaml: string(buffer[:]),
-		NodeNames:    args,
-	}
-
-	return apinode.NodeAdd(&set)
 }

internal/app/wwctl/node/add/main_test.go

@@ -2,7 +2,6 @@ package add
 
 import (
 	"bytes"
-	"fmt"
 	"testing"
 
 	"github.com/hpcng/warewulf/internal/pkg/node"
@@ -12,29 +11,12 @@ import (
 )
 
 func Test_Add(t *testing.T) {
-	t.Helper()
-	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)
-	warewulfd.SetNoDaemon()
-	buf := new(bytes.Buffer)
-	baseCmd.SetOut(buf)
-	baseCmd.SetErr(buf)
 	tests := []struct {
 		name    string
 		args    []string
 		wantErr bool
 		stdout  string
 		outDb   string
-		flags   map[string]string
 	}{
 		{name: "single node add",
 			args:    []string{"n01"},
@@ -48,8 +30,7 @@ nodes:
     - default
 `},
 		{name: "single node add, profile foo",
-			args:    []string{"n01"},
+			args:    []string{"--profile=foo", "n01"},
-			flags:   map[string]string{"profile": "foo"},
 			wantErr: false,
 			stdout:  "",
 			outDb: `WW_INTERNAL: 43
@@ -60,8 +41,7 @@ nodes:
     - foo
 `},
 		{name: "single node add with Kernel args",
-			args:    []string{"n01"},
+			args:    []string{"--kernelargs=foo", "n01"},
-			flags:   map[string]string{"kernelargs": "foo"},
 			wantErr: false,
 			stdout:  "",
 			outDb: `WW_INTERNAL: 43
@@ -87,9 +67,8 @@ nodes:
     profiles:
     - default
 `},
-		{name: "single node with ipaddr",
+		{name: "single node with ipaddr6",
-			args:    []string{"n01"},
+			args:    []string{"--ipaddr6=fdaa::1", "n01"},
-			flags:   map[string]string{"ipaddr": "10.10.0.1"},
 			wantErr: false,
 			stdout:  "",
 			outDb: `WW_INTERNAL: 43
@@ -100,11 +79,24 @@ nodes:
     - default
     network devices:
       default:
-        ipaddr: 10.10.0.1
+        ip6addr: fdaa::1
+`},
+		{name: "single node with ipaddr",
+			args:    []string{"--ipaddr=10.0.0.1", "n01"},
+			wantErr: false,
+			stdout:  "",
+			outDb: `WW_INTERNAL: 43
+nodeprofiles: {}
+nodes:
+  n01:
+    profiles:
+    - default
+    network devices:
+      default:
+        ipaddr: 10.0.0.1
 `},
 		{name: "three nodes with ipaddr",
-			args:    []string{"n[01-02,03]"},
+			args:    []string{"--ipaddr=10.10.0.1", "n[01-02,03]"},
-			flags:   map[string]string{"ipaddr": "10.10.0.1"},
 			wantErr: false,
 			stdout:  "",
 			outDb: `WW_INTERNAL: 43
@@ -130,8 +122,7 @@ nodes:
         ipaddr: 10.10.0.3
 `},
 		{name: "three nodes with ipaddr different network",
-			args:    []string{"n[01-03]"},
+			args:    []string{"--ipaddr=10.10.0.1", "--netname=foo", "n[01-03]"},
-			flags:   map[string]string{"ipaddr": "10.10.0.1", "netname": "foo"},
 			wantErr: false,
 			stdout:  "",
 			outDb: `WW_INTERNAL: 43
@@ -157,8 +148,7 @@ nodes:
         ipaddr: 10.10.0.3
 `},
 		{name: "three nodes with ipaddr different network, with ipmiaddr",
-			args:    []string{"n[01-03]"},
+			args:    []string{"--ipaddr=10.10.0.1", "--netname=foo", "--ipmiaddr=10.20.0.1", "n[01-03]"},
-			flags:   map[string]string{"ipaddr": "10.10.0.1", "netname": "foo", "ipmiaddr": "10.20.0.1"},
 			wantErr: false,
 			stdout:  "",
 			outDb: `WW_INTERNAL: 43
@@ -190,41 +180,42 @@ 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)
+	warewulfd.SetNoDaemon()
 	for _, tt := range tests {
 		db, err = node.TestNew([]byte(nodes_yml))
 		assert.NoError(t, err)
-		fmt.Printf("Running test: %s\n", tt.name)
+		t.Logf("Running test: %s\n", tt.name)
 		t.Run(tt.name, func(t *testing.T) {
-			// store global NodeConf as the NodeConf.NetDevs["default"] will be delete
+			baseCmd := GetCommand()
-			// in the main.go
-			tmpConfNet := NodeConf.NetDevs["default"]
-			//tmpConf := NodeConf
-			//tmpKernel := NodeConf.Kernel
 			baseCmd.SetArgs(tt.args)
-			for key, val := range tt.flags {
+			buf := new(bytes.Buffer)
-				baseCmd.Flags().Set(key, val)
+			baseCmd.SetOut(buf)
-			}
+			baseCmd.SetErr(buf)
 			err = baseCmd.Execute()
 			if (err != nil) != tt.wantErr {
 				t.Errorf("Got unwanted error: %s", err)
-				return
+				t.FailNow()
 			}
 			dump := string(db.DBDump())
 			if dump != tt.outDb {
 				t.Errorf("DB dump is wrong, got:'%s'\nwant:'%s'", dump, tt.outDb)
-				return
+				t.FailNow()
 			}
 			if buf.String() != tt.stdout {
 				t.Errorf("Got wrong output, got:'%s'\nwant:'%s'", buf.String(), tt.stdout)
-				return
+				t.FailNow()
 			}
-			NodeConf.NetDevs["default"] = tmpConfNet
-			for key, _ := range tt.flags {
-				baseCmd.Flags().Set(key, "hark")
-			}
-
-			//NodeConf = tmpConf
-			//NodeConf.Kernel = node.NewConf().Kernel
 		})
 	}
 }

internal/app/wwctl/node/add/root.go

@@ -10,23 +10,26 @@ import (
 	"github.com/spf13/cobra"
 )
 
-var (
+// Holds the variables which are needed in CobraRunE
-	baseCmd = &cobra.Command{
+type variables struct {
+	netName  string
+	nodeConf node.NodeConf
+}
+
+// Returns the newly created command
+func GetCommand() *cobra.Command {
+	vars := variables{}
+	vars.nodeConf = node.NewConf()
+	baseCmd := &cobra.Command{
 		DisableFlagsInUseLine: true,
 		Use:                   "add [OPTIONS] NODENAME",
 		Short:                 "Add new node to Warewulf",
 		Long:                  "This command will add a new node named NODENAME to Warewulf.",
-		RunE:                  CobraRunE,
+		RunE:                  CobraRunE(&vars),
 		Args:                  cobra.MinimumNArgs(1),
 	}
-	NodeConf node.NodeConf
+	vars.nodeConf.CreateFlags(baseCmd, []string{"tagdel", "nettagdel", "ipmitagdel"})
-	NetName  string
+	baseCmd.PersistentFlags().StringVar(&vars.netName, "netname", "", "Set network name for network options")
-)
-
-func init() {
-	NodeConf = node.NewConf()
-	NodeConf.CreateFlags(baseCmd, []string{})
-	baseCmd.PersistentFlags().StringVar(&NetName, "netname", "", "Set network name for network options")
 
 	// register the command line completions
 	if err := baseCmd.RegisterFlagCompletionFunc("container", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
@@ -65,9 +68,6 @@ func init() {
 		log.Println(err)
 	}
 
-}
+	// GetRootCommand returns the root cobra.Command for the application.
-
-// GetRootCommand returns the root cobra.Command for the application.
-func GetCommand() *cobra.Command {
 	return baseCmd
 }