Merge remote-tracking branch 'jason/issue/658' into development

Signed-off-by: Jonathon Anderson <janderson@ciq.co>

CHANGELOG.md

@@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - All paths can now be configured in `warewulf.conf`, check the paths section of of 
    `wwctl --emptyconf genconfig warewulfconf print` for the available paths.
 - Added experimental dnsmasq support.
+- Refactored `profile add` command to make it alike `node add`. #658 #659 
 
 - new subcommand `wwctl genconf` is available with following subcommands:
   * `completions` which will create the files used for bash-completion. Also

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

@@ -257,6 +257,7 @@ nodes:
 		assert.NoError(t, err)
 		assert.NoError(t, tempNodeConf.Truncate(0))
 		_, err = tempNodeConf.Write([]byte(nodes_yml))
+		assert.NoError(t, err)
 		assert.NoError(t, tempNodeConf.Sync())
 		t.Logf("Running test: %s\n", tt.name)
 		t.Run(tt.name, func(t *testing.T) {

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

@@ -55,6 +55,7 @@ nodes:
 		assert.NoError(t, err)
 		assert.NoError(t, tempNodeConf.Truncate(0))
 		_, err = tempNodeConf.Write([]byte(tt.inDb))
+		assert.NoError(t, err)
 		assert.NoError(t, tempNodeConf.Sync())
 		t.Logf("Running test: %s\n", tt.name)
 		t.Run(tt.name, func(t *testing.T) {

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

@@ -13,46 +13,48 @@ import (
 	"github.com/spf13/cobra"
 )
 
-func CobraRunE(cmd *cobra.Command, args []string) (err error) {
-	// run converters for different types
-	for _, c := range Converters {
-		if err := c(); err != nil {
-			return err
+func CobraRunE(vars *variables) func(cmd *cobra.Command, args []string) (err error) {
+	return func(cmd *cobra.Command, args []string) (err error) {
+		// run converters for different types
+		for _, c := range Converters {
+			if err := c(); err != nil {
+				return err
+			}
+		}
+		// remove the default network as the all network values are assigned
+		// to this network
+		if vars.netName != "" {
+			netDev := *vars.profileConf.NetDevs["default"]
+			vars.profileConf.NetDevs[vars.netName] = &netDev
+			delete(vars.profileConf.NetDevs, "default")
 		}
-	}
-	// remove the default network as the all network values are assigned
-	// to this network
-	if NetName != "" {
-		netDev := *ProfileConf.NetDevs["default"]
-		ProfileConf.NetDevs[NetName] = &netDev
-		delete(ProfileConf.NetDevs, "default")
 
-	}
-	buffer, err := yaml.Marshal(ProfileConf)
-	if err != nil {
-		wwlog.Error("Cant marshall nodeInfo", err)
-		os.Exit(1)
-	}
-	set := wwapiv1.NodeSetParameter{
-		NodeConfYaml: string(buffer[:]),
-		NetdevDelete: SetNetDevDel,
-		AllNodes:     SetNodeAll,
-		Force:        SetForce,
-		NodeNames:    args,
-	}
-
-	if !SetYes {
-		// The checks run twice in the prompt case.
-		// Avoiding putting in a blocking prompt in an API.
-		_, _, err = apiprofile.ProfileSetParameterCheck(&set, false)
+		buffer, err := yaml.Marshal(vars.profileConf)
 		if err != nil {
-			return
+			wwlog.Error("Cant marshall nodeInfo", err)
+			os.Exit(1)
+		}
+		set := wwapiv1.NodeSetParameter{
+			NodeConfYaml: string(buffer[:]),
+			NetdevDelete: SetNetDevDel,
+			AllNodes:     SetNodeAll,
+			Force:        SetForce,
+			NodeNames:    args,
 		}
 
-		yes := util.ConfirmationPrompt(fmt.Sprintf("Are you sure you add the profile %s", args))
-		if !yes {
-			return
+		if !SetYes {
+			// The checks run twice in the prompt case.
+			// Avoiding putting in a blocking prompt in an API.
+			_, _, err = apiprofile.ProfileSetParameterCheck(&set, false)
+			if err != nil {
+				return
+			}
+
+			yes := util.ConfirmationPrompt(fmt.Sprintf("Are you sure you add the profile %s", args))
+			if !yes {
+				return
+			}
 		}
+		return apiprofile.AddProfile(&set)
 	}
-	return apiprofile.AddProfile(&set, false)
 }

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

@@ -0,0 +1,100 @@
+package add
+
+import (
+	"bytes"
+	"os"
+	"testing"
+
+	"github.com/hpcng/warewulf/internal/pkg/node"
+	"github.com/hpcng/warewulf/internal/pkg/warewulfconf"
+	"github.com/hpcng/warewulf/internal/pkg/warewulfd"
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_Add(t *testing.T) {
+	tests := []struct {
+		name    string
+		args    []string
+		wantErr bool
+		stdout  string
+		outDb   string
+	}{
+		{
+			name:    "single profile add",
+			args:    []string{"--yes", "p01"},
+			wantErr: false,
+			stdout:  "",
+			outDb: `WW_INTERNAL: 43
+nodeprofiles:
+  p01:
+    network devices:
+      default: {}
+nodes: {}
+`,
+		},
+		{
+			name:    "single profile add with netname and netdev",
+			args:    []string{"--yes", "--netname", "primary", "--netdev", "eno3", "p02"},
+			wantErr: false,
+			stdout:  "",
+			outDb: `WW_INTERNAL: 43
+nodeprofiles:
+  p02:
+    network devices:
+      primary:
+        device: eno3
+nodes: {}
+`,
+		},
+	}
+
+	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 {
+		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, err)
+		assert.NoError(t, tempNodeConf.Sync())
+		t.Logf("Running test: %s\n", tt.name)
+		t.Run(tt.name, func(t *testing.T) {
+			baseCmd := GetCommand()
+			baseCmd.SetArgs(tt.args)
+			buf := new(bytes.Buffer)
+			baseCmd.SetOut(buf)
+			baseCmd.SetErr(buf)
+			err = baseCmd.Execute()
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Got unwanted error: %s", err)
+				t.FailNow()
+			}
+			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()
+			}
+			if buf.String() != tt.stdout {
+				t.Errorf("Got wrong output, got:'%s'\nwant:'%s'", buf.String(), tt.stdout)
+				t.FailNow()
+			}
+		})
+	}
+}

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

@@ -10,15 +10,12 @@ import (
 	"github.com/spf13/cobra"
 )
 
+type variables struct {
+	netName     string
+	profileConf node.NodeConf
+}
+
 var (
-	baseCmd = &cobra.Command{
-		DisableFlagsInUseLine: true,
-		Use:                   "add PROFILE",
-		Short:                 "Add a new node profile",
-		Long:                  "This command adds a new named PROFILE.",
-		RunE:                  CobraRunE,
-		Args:                  cobra.ExactArgs(1),
-	}
 	SetNetDevDel string
 	SetNodeAll   bool
 	SetYes       bool
@@ -30,10 +27,19 @@ var (
 
 // GetRootCommand returns the root cobra.Command for the application.
 func GetCommand() *cobra.Command {
-	ProfileConf = node.NewConf()
-	Converters = ProfileConf.CreateFlags(baseCmd,
+	vars := variables{}
+	vars.profileConf = node.NewConf()
+	baseCmd := &cobra.Command{
+		DisableFlagsInUseLine: true,
+		Use:                   "add PROFILE",
+		Short:                 "Add a new node profile",
+		Long:                  "This command adds a new named PROFILE.",
+		RunE:                  CobraRunE(&vars),
+		Args:                  cobra.ExactArgs(1),
+	}
+	Converters = vars.profileConf.CreateFlags(baseCmd,
 		[]string{"ipaddr", "ipaddr6", "ipmiaddr", "profile"})
-	baseCmd.PersistentFlags().StringVar(&NetName, "netname", "", "Set network name for network options")
+	baseCmd.PersistentFlags().StringVar(&vars.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) {
 		list, _ := container.ListSources()

internal/app/wwctl/profile/list/main_test.go

@@ -56,6 +56,7 @@ nodes:
 		assert.NoError(t, err)
 		assert.NoError(t, tempNodeConf.Truncate(0))
 		_, err = tempNodeConf.Write([]byte(tt.inDb))
+		assert.NoError(t, err)
 		assert.NoError(t, tempNodeConf.Sync())
 		assert.NoError(t, err)
 		t.Logf("Running test: %s\n", tt.name)

internal/pkg/api/profile/profile.go

@@ -126,22 +126,41 @@ func ProfileSetParameterCheck(set *wwapiv1.NodeSetParameter, console bool) (node
 /*
 Adds a new profile with the given name
 */
-func AddProfile(set *wwapiv1.NodeSetParameter, console bool) error {
+
+func AddProfile(nsp *wwapiv1.NodeSetParameter) error {
+	if nsp == nil {
+		return fmt.Errorf("NodeSetParameter is nill")
+	}
+
 	nodeDB, err := node.New()
 	if err != nil {
 		return errors.Wrap(err, "Could not open database")
 	}
 
-	if util.InSlice(nodeDB.ListAllProfiles(), set.NodeNames[0]) {
-		return errors.New(fmt.Sprintf("profile with name %s allready exists", set.NodeNames[0]))
+	if util.InSlice(nodeDB.ListAllProfiles(), nsp.NodeNames[0]) {
+		return errors.New(fmt.Sprintf("profile with name %s allready exists", nsp.NodeNames[0]))
 	}
-	_, err = nodeDB.AddProfile(set.NodeNames[0])
+
+	var nodeConf node.NodeConf
+	err = yaml.Unmarshal([]byte(nsp.NodeConfYaml), &nodeConf)
 	if err != nil {
-		return errors.Wrap(err, "Could not create new profile")
+		return errors.Wrap(err, "failed to decode nodeConf")
 	}
-	err = apinode.DbSave(&nodeDB)
+
+	n, err := nodeDB.AddProfile(nsp.NodeNames[0])
 	if err != nil {
-		return errors.Wrap(err, "Could not persist new profile")
+		return errors.Wrap(err, "failed to add node")
+	}
+	n.SetFrom(&nodeConf)
+
+	err = nodeDB.ProfileUpdate(n)
+	if err != nil {
+		return errors.Wrap(err, "failed to update nodedb")
+	}
+
+	err = nodeDB.Persist()
+	if err != nil {
+		return errors.Wrap(err, "failed to persist new profile")
 	}
 	return nil
 }

internal/pkg/node/constructors.go

@@ -14,8 +14,10 @@ import (
 	"gopkg.in/yaml.v2"
 )
 
-var ConfigFile string
-var DefaultConfig string
+var (
+	ConfigFile    string
+	DefaultConfig string
+)
 
 // used as fallback if DefaultConfig can't be read
 var FallBackConf = `---

internal/pkg/node/modifiers.go

@@ -38,7 +38,6 @@ func (config *NodeYaml) AddNode(nodeID string) (NodeInfo, error) {
 }
 
 func (config *NodeYaml) DelNode(nodeID string) error {
-
 	if _, ok := config.Nodes[nodeID]; !ok {
 		return errors.New("Nodename does not exist: " + nodeID)
 	}
@@ -76,14 +75,14 @@ func (config *NodeYaml) AddProfile(profileID string) (NodeInfo, error) {
 	}
 
 	config.NodeProfiles[profileID] = &node
-
+	config.NodeProfiles[profileID].NetDevs = make(map[string]*NetDevs)
 	n.Id.Set(profileID)
+	n.NetDevs = make(map[string]*NetDevEntry)
 
 	return n, nil
 }
 
 func (config *NodeYaml) DelProfile(profileID string) error {
-
 	if _, ok := config.NodeProfiles[profileID]; !ok {
 		return errors.New("Profile does not exist: " + profileID)
 	}
@@ -121,7 +120,7 @@ func (config *NodeYaml) Persist() error {
 		wwlog.Error("%s", dumpErr)
 		os.Exit(1)
 	}
-	file, err := os.OpenFile(ConfigFile, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644)
+	file, err := os.OpenFile(ConfigFile, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o644)
 	if err != nil {
 		wwlog.Error("%s", err)
 		os.Exit(1)