From 0faada6aae52c68efcf7d4dea5856d0636708af6 Mon Sep 17 00:00:00 2001 From: jason yang Date: Wed, 12 Apr 2023 10:27:44 +0000 Subject: [PATCH] refactor the profile add Signed-off-by: jason yang --- CHANGELOG.md | 1 + .../app/wwctl/container/list/main_test.go | 13 +-- internal/app/wwctl/profile/add/main.go | 74 ++++++++-------- internal/app/wwctl/profile/add/main_test.go | 87 +++++++++++++++++++ internal/app/wwctl/profile/add/root.go | 28 +++--- internal/pkg/api/profile/profile.go | 33 +++++-- internal/pkg/node/constructors.go | 9 +- internal/pkg/node/modifiers.go | 7 +- 8 files changed, 180 insertions(+), 72 deletions(-) create mode 100644 internal/app/wwctl/profile/add/main_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index dac937e9..405996e4 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/internal/app/wwctl/container/list/main_test.go b/internal/app/wwctl/container/list/main_test.go index fadfab81..8a6b4b1d 100644 --- a/internal/app/wwctl/container/list/main_test.go +++ b/internal/app/wwctl/container/list/main_test.go @@ -24,11 +24,9 @@ func Test_List(t *testing.T) { mockFunc func() }{ { - name: "profile list test", - args: []string{}, - stdout: `CONTAINERNAMENODESKERNELVERSIONCREATIONTIMEMODIFICATIONTIMESIZE -test1kernel01Jan7000:00UTC01Jan7000:00UTC1B - `, + name: "container list test", + args: []string{}, + stdout: `test 1 kernel`, inDb: `WW_INTERNAL: 43 nodeprofiles: default: {} @@ -87,11 +85,8 @@ WW_INTERNAL: 0 stdoutW.Close() stdout := <-stdoutC - stdout = strings.TrimSpace(stdout) - stdout = strings.ReplaceAll(stdout, " ", "") assert.NotEmpty(t, stdout, "os.stdout should not be empty") - tt.stdout = strings.ReplaceAll(strings.TrimSpace(tt.stdout), " ", "") - if stdout != strings.ReplaceAll(strings.TrimSpace(tt.stdout), " ", "") { + if !strings.Contains(stdout, tt.stdout) { t.Errorf("Got wrong output, got:\n '%s'\n, but want:\n '%s'\n", stdout, tt.stdout) t.FailNow() } diff --git a/internal/app/wwctl/profile/add/main.go b/internal/app/wwctl/profile/add/main.go index dc23d8fc..40ad46a4 100644 --- a/internal/app/wwctl/profile/add/main.go +++ b/internal/app/wwctl/profile/add/main.go @@ -13,50 +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.AddProfile(&set, false) + buffer, err := yaml.Marshal(vars.profileConf) if err != nil { - return + wwlog.Error("Cant marshall nodeInfo", err) + os.Exit(1) } - _, _, err = apiprofile.ProfileSetParameterCheck(&set, false) - if err != nil { - return + 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.ProfileSet(&set) } diff --git a/internal/app/wwctl/profile/add/main_test.go b/internal/app/wwctl/profile/add/main_test.go new file mode 100644 index 00000000..47c7df1c --- /dev/null +++ b/internal/app/wwctl/profile/add/main_test.go @@ -0,0 +1,87 @@ +package add + +import ( + "bytes" + "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 + ` + 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) + 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() + } + dump := string(db.DBDump()) + 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() + } + }) + } +} diff --git a/internal/app/wwctl/profile/add/root.go b/internal/app/wwctl/profile/add/root.go index 6edf403d..3a0d4336 100644 --- a/internal/app/wwctl/profile/add/root.go +++ b/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() diff --git a/internal/pkg/api/profile/profile.go b/internal/pkg/api/profile/profile.go index 15fc4512..5b0a50c3 100644 --- a/internal/pkg/api/profile/profile.go +++ b/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 } diff --git a/internal/pkg/node/constructors.go b/internal/pkg/node/constructors.go index 578cb094..3f400073 100644 --- a/internal/pkg/node/constructors.go +++ b/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 +) var cachedDB NodeYaml @@ -91,7 +93,8 @@ func New() (NodeYaml, error) { wwlog.Debug("Returning node object") cachedDB = ret cachedDB.current = true - return ret, nil + cachedDB.persist = true + return cachedDB, nil } /* diff --git a/internal/pkg/node/modifiers.go b/internal/pkg/node/modifiers.go index 2c644657..babe11f5 100644 --- a/internal/pkg/node/modifiers.go +++ b/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) } @@ -131,7 +130,7 @@ func (config *NodeYaml) Persist() error { if !config.persist { return err } - 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)