diff --git a/CHANGELOG.md b/CHANGELOG.md index 87cae42a..5183379f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed the ability to set MTU with wwctl #947 - Fixed a bug where profile tags were erroneously overridden by empty node values. #884 +- Fixed bug where tags from profiles weren't rendered #967 ### Changed diff --git a/internal/app/wwctl/overlay/list/main.go b/internal/app/wwctl/overlay/list/main.go index ac02a619..daab2a93 100644 --- a/internal/app/wwctl/overlay/list/main.go +++ b/internal/app/wwctl/overlay/list/main.go @@ -1,7 +1,6 @@ package list import ( - "fmt" "os" "syscall" @@ -26,9 +25,9 @@ func CobraRunE(cmd *cobra.Command, args []string) error { } if ListLong { - fmt.Printf("%-10s %5s %-5s %-18s %s\n", "PERM MODE", "UID", "GID", "SYSTEM-OVERLAY", "FILE PATH") + wwlog.Info("%-10s %5s %-5s %-18s %s\n", "PERM MODE", "UID", "GID", "SYSTEM-OVERLAY", "FILE PATH") } else { - fmt.Printf("%-30s %-12s\n", "OVERLAY NAME", "FILES/DIRS") + wwlog.Info("%-30s %-12s\n", "OVERLAY NAME", "FILES/DIRS") } for o := range overlays { @@ -51,19 +50,19 @@ func CobraRunE(cmd *cobra.Command, args []string) error { sys := s.Sys() - fmt.Printf("%v %5d %-5d %-18s /%s\n", perms, sys.(*syscall.Stat_t).Uid, sys.(*syscall.Stat_t).Gid, overlays[o], files[file]) + wwlog.Info("%v %5d %-5d %-18s /%s\n", perms, sys.(*syscall.Stat_t).Uid, sys.(*syscall.Stat_t).Gid, overlays[o], files[file]) } } else if ListContents { var fileCount int for file := range files { - fmt.Printf("%-30s /%-12s\n", name, files[file]) + wwlog.Info("%-30s /%-12s\n", name, files[file]) fileCount++ } if fileCount == 0 { - fmt.Printf("%-30s %-12d\n", name, 0) + wwlog.Info("%-30s %-12d\n", name, 0) } } else { - fmt.Printf("%-30s %-12d\n", name, len(files)) + wwlog.Info("%-30s %-12d\n", name, len(files)) } } else { diff --git a/internal/app/wwctl/overlay/list/main_test.go b/internal/app/wwctl/overlay/list/main_test.go new file mode 100644 index 00000000..7fdbb694 --- /dev/null +++ b/internal/app/wwctl/overlay/list/main_test.go @@ -0,0 +1,43 @@ +package list + +import ( + "bytes" + "path" + "testing" + + "github.com/hpcng/warewulf/internal/pkg/wwlog" + "github.com/stretchr/testify/assert" + + "github.com/hpcng/warewulf/internal/pkg/testenv" + "github.com/hpcng/warewulf/internal/pkg/warewulfd" +) + +func Test_Overlay_List(t *testing.T) { + env := testenv.New(t) + env.WriteFile(t, path.Join(testenv.WWOverlaydir, "testoverlay/email.ww"), ` +{{ if .Tags.email }}eMail: {{ .Tags.email }}{{else}} noMail{{- end }} +`) + defer env.RemoveAll(t) + warewulfd.SetNoDaemon() + t.Run("overlay list", func(t *testing.T) { + baseCmd := GetCommand() + buf := new(bytes.Buffer) + baseCmd.SetOut(buf) + baseCmd.SetErr(buf) + wwlog.SetLogWriter(buf) + err := baseCmd.Execute() + assert.NoError(t, err) + assert.Contains(t, buf.String(), "testoverlay") + }) + t.Run("overlay list all", func(t *testing.T) { + baseCmd.SetArgs([]string{"-a"}) + baseCmd := GetCommand() + buf := new(bytes.Buffer) + baseCmd.SetOut(buf) + baseCmd.SetErr(buf) + wwlog.SetLogWriter(buf) + err := baseCmd.Execute() + assert.NoError(t, err) + assert.Contains(t, buf.String(), "email.ww") + }) +} diff --git a/internal/app/wwctl/overlay/show/main.go b/internal/app/wwctl/overlay/show/main.go index 8475c85e..f201e548 100644 --- a/internal/app/wwctl/overlay/show/main.go +++ b/internal/app/wwctl/overlay/show/main.go @@ -3,7 +3,6 @@ package show import ( "bufio" "bytes" - "fmt" "os" "path" "path/filepath" @@ -43,7 +42,7 @@ func CobraRunE(cmd *cobra.Command, args []string) error { os.Exit(1) } - fmt.Print(string(f)) + wwlog.Info(string(f)) } else { if !util.IsFile(overlayFile) { wwlog.Debug("%s is not a file", overlayFile) @@ -105,7 +104,7 @@ func CobraRunE(cmd *cobra.Command, args []string) error { wwlog.Info("backupFile: %v\nwriteFile: %v", backupFile, writeFile) wwlog.Info("Filename: %s\n", destFileName) } - fmt.Print(outBuffer.String()) + wwlog.Info(outBuffer.String()) } return nil } diff --git a/internal/app/wwctl/overlay/show/main_test.go b/internal/app/wwctl/overlay/show/main_test.go new file mode 100644 index 00000000..de8db114 --- /dev/null +++ b/internal/app/wwctl/overlay/show/main_test.go @@ -0,0 +1,87 @@ +package show + +import ( + "bytes" + "path" + "testing" + + "github.com/hpcng/warewulf/internal/pkg/wwlog" + "github.com/stretchr/testify/assert" + + "github.com/hpcng/warewulf/internal/pkg/testenv" + "github.com/hpcng/warewulf/internal/pkg/warewulfd" +) + +var ( + overlayCont = ` +{{ if .Tags.email }}eMail: {{ .Tags.email }}{{else}} noMail{{- end }} +` +) + +func Test_Overlay_List(t *testing.T) { + env := testenv.New(t) + env.WriteFile(t, "etc/warewulf/nodes.conf", + `WW_INTERNAL: 43 +nodeprofiles: + default: + tags: + email: admin@localhost + empty: {} +nodes: + node1: + tags: + email: admin@node1 + node2: {} + node3: + profiles: + - empty +`) + + env.WriteFile(t, path.Join(testenv.WWOverlaydir, "testoverlay/email.ww"), overlayCont) + defer env.RemoveAll(t) + warewulfd.SetNoDaemon() + t.Run("overlay show raw", func(t *testing.T) { + baseCmd.SetArgs([]string{"testoverlay", "email.ww"}) + baseCmd := GetCommand() + buf := new(bytes.Buffer) + baseCmd.SetOut(buf) + baseCmd.SetErr(buf) + wwlog.SetLogWriter(buf) + err := baseCmd.Execute() + assert.NoError(t, err) + assert.Contains(t, buf.String(), overlayCont) + }) + t.Run("overlay show rendered node tag", func(t *testing.T) { + baseCmd.SetArgs([]string{"-r", "node1", "testoverlay", "email.ww"}) + baseCmd := GetCommand() + buf := new(bytes.Buffer) + baseCmd.SetOut(buf) + baseCmd.SetErr(buf) + wwlog.SetLogWriter(buf) + err := baseCmd.Execute() + assert.NoError(t, err) + assert.Contains(t, buf.String(), "admin@node1") + }) + t.Run("overlay show rendered profile tag", func(t *testing.T) { + baseCmd.SetArgs([]string{"-r", "node2", "testoverlay", "email.ww"}) + baseCmd := GetCommand() + buf := new(bytes.Buffer) + baseCmd.SetOut(buf) + baseCmd.SetErr(buf) + wwlog.SetLogWriter(buf) + err := baseCmd.Execute() + assert.NoError(t, err) + assert.Contains(t, buf.String(), "admin@localhost") + }) + t.Run("overlay show no tag", func(t *testing.T) { + baseCmd.SetArgs([]string{"-r", "node3", "testoverlay", "email.ww"}) + baseCmd := GetCommand() + buf := new(bytes.Buffer) + baseCmd.SetOut(buf) + baseCmd.SetErr(buf) + wwlog.SetLogWriter(buf) + err := baseCmd.Execute() + assert.NoError(t, err) + assert.Contains(t, buf.String(), "noMail") + }) +} diff --git a/internal/pkg/node/transformer_test.go b/internal/pkg/node/transformer_test.go index 0f8746fe..805ff6b3 100644 --- a/internal/pkg/node/transformer_test.go +++ b/internal/pkg/node/transformer_test.go @@ -5,6 +5,7 @@ import ( "strconv" "testing" + "github.com/stretchr/testify/assert" "gopkg.in/yaml.v2" ) @@ -57,6 +58,9 @@ nodes: ipaddr: 1.1.1.1 tags: foo: foo ipmi node3 + test_node4: + profiles: + - profile2 ` var ret NodeYaml _ = yaml.Unmarshal([]byte(data), &ret) @@ -69,6 +73,7 @@ func Test_nodeYaml_SetFrom(t *testing.T) { test_node2 := NewInfo() test_node3 := NewInfo() test_node4 := NewInfo() + test_empty := NewInfo() for _, n := range nodes { if n.Id.Get() == "test_node1" { test_node1 = n @@ -79,6 +84,9 @@ func Test_nodeYaml_SetFrom(t *testing.T) { if n.Id.Get() == "test_node3" { test_node3 = n } + if n.Id.Get() == "test_node4" { + test_node4 = n + } } getByNametests := []struct { name string @@ -202,15 +210,11 @@ func Test_nodeYaml_SetFrom(t *testing.T) { }) t.Run("Get() tag foo from node, tag present in profile", func(t *testing.T) { value := test_node3.Tags["foo"].Get() - if value != "foo node3" { - t.Errorf("Get() returned wrong tag for foo: %s", value) - } + assert.Equal(t, "foo node3", value, "wrong value for tag") }) t.Run("Get() tag foobaar from node", func(t *testing.T) { value := test_node3.Tags["foobaar"].Get() - if value != "foobaar node3" { - t.Errorf("Get() returned wrong tag for foo: %s", value) - } + assert.Equal(t, "foobaar node3", value, "wrong value for tag") }) t.Run("Get() ipmitag foo from profile, node does not have this tag", func(t *testing.T) { value := test_node3.Ipmi.Tags["foo"].Get() @@ -218,10 +222,14 @@ func Test_nodeYaml_SetFrom(t *testing.T) { t.Errorf("Get() returned wrong tag for foo: %s", value) } }) + t.Run("Get() tag foo from profile, node does not have this tag", func(t *testing.T) { + value := test_node4.Tags["foo"].Get() + assert.Equal(t, "foo profile2", value, "wrong value for tag") + }) t.Run("Set() comment foo for empty node", func(t *testing.T) { - test_node4.Comment.Set("foo") + test_empty.Comment.Set("foo") nodeConf := NewConf() - nodeConf.GetFrom(test_node4) + nodeConf.GetFrom(test_empty) ymlByte, _ := yaml.Marshal(nodeConf) wanted := `comment: foo kernel: {} @@ -232,8 +240,8 @@ ipmi: {} } // have to remove the comment for further tests, as vscode // can test single functions - test_node4.Comment.Set("UNDEF") - nodeConf.GetFrom(test_node4) + test_empty.Comment.Set("UNDEF") + nodeConf.GetFrom(test_empty) nodeConf.Flatten() ymlByte, _ = yaml.Marshal(nodeConf) wanted = `{} @@ -244,9 +252,9 @@ ipmi: {} }) t.Run("Set() ipmiuser foo for flattened empty node", func(t *testing.T) { - test_node4.Ipmi.UserName.Set("foo") + test_empty.Ipmi.UserName.Set("foo") nodeConf := NewConf() - nodeConf.GetFrom(test_node4) + nodeConf.GetFrom(test_empty) nodeConf.Flatten() ymlByte, _ := yaml.Marshal(nodeConf) wanted := `ipmi: @@ -255,9 +263,9 @@ ipmi: {} if !(wanted == string(ymlByte)) { t.Errorf("Got wrong yml, wanted:\n'%s'\nGot:\n'%s'", wanted, string(ymlByte)) } - test_node4.Ipmi.Tags["foo"] = &Entry{} - test_node4.Ipmi.Tags["foo"].Set("baar") - nodeConf.GetFrom(test_node4) + test_empty.Ipmi.Tags["foo"] = &Entry{} + test_empty.Ipmi.Tags["foo"].Set("baar") + nodeConf.GetFrom(test_empty) nodeConf.Flatten() ymlByte, _ = yaml.Marshal(nodeConf) wanted = `ipmi: @@ -268,13 +276,13 @@ ipmi: {} if !(wanted == string(ymlByte)) { t.Errorf("Got wrong yml, wanted:\n'%s'\nGot:\n'%s'", wanted, string(ymlByte)) } - test_node4.Ipmi.UserName.Set("UNSET") - delete(test_node4.Ipmi.Tags, "foo") + test_empty.Ipmi.UserName.Set("UNSET") + delete(test_empty.Ipmi.Tags, "foo") }) t.Run("Set() kernelargs foo for flattened empty node", func(t *testing.T) { - test_node4.Kernel.Args.Set("foo") + test_empty.Kernel.Args.Set("foo") nodeConf := NewConf() - nodeConf.GetFrom(test_node4) + nodeConf.GetFrom(test_empty) nodeConf.Flatten() ymlByte, _ := yaml.Marshal(nodeConf) wanted := `kernel: @@ -283,13 +291,13 @@ ipmi: {} if !(wanted == string(ymlByte)) { t.Errorf("Got wrong yml, wanted:\n'%s'\nGot:\n'%s'", wanted, string(ymlByte)) } - test_node4.Kernel.Args.Set("--") + test_empty.Kernel.Args.Set("--") }) t.Run("Set() tag foo to bar for flattened empty node", func(t *testing.T) { - test_node4.Tags["foo"] = &Entry{} - test_node4.Tags["foo"].Set("baar") + test_empty.Tags["foo"] = &Entry{} + test_empty.Tags["foo"].Set("baar") nodeConf := NewConf() - nodeConf.GetFrom(test_node4) + nodeConf.GetFrom(test_empty) nodeConf.Flatten() ymlByte, _ := yaml.Marshal(nodeConf) wanted := `tags: @@ -298,9 +306,9 @@ ipmi: {} if !(wanted == string(ymlByte)) { t.Errorf("Got wrong yml, wanted:\n'%s'\nGot:\n'%s'", wanted, string(ymlByte)) } - delete(test_node4.Tags, "foo") + delete(test_empty.Tags, "foo") nodeConf = NewConf() - nodeConf.GetFrom(test_node4) + nodeConf.GetFrom(test_empty) nodeConf.Flatten() ymlByte, _ = yaml.Marshal(nodeConf) wanted = `{} @@ -311,10 +319,10 @@ ipmi: {} }) t.Run("Set() netdev foo with device name baar for flattened empty node", func(t *testing.T) { - test_node4.NetDevs["foo"] = new(NetDevEntry) - test_node4.NetDevs["foo"].Device.Set("baar") + test_empty.NetDevs["foo"] = new(NetDevEntry) + test_empty.NetDevs["foo"].Device.Set("baar") nodeConf := NewConf() - nodeConf.GetFrom(test_node4) + nodeConf.GetFrom(test_empty) nodeConf.Flatten() ymlByte, _ := yaml.Marshal(nodeConf) wanted := `network devices: @@ -324,10 +332,10 @@ ipmi: {} if !(wanted == string(ymlByte)) { t.Errorf("Got wrong yml, wanted:\n'%s'\nGot:\n'%s'", wanted, string(ymlByte)) } - test_node4.NetDevs["foo"].Tags = make(map[string]*Entry) - test_node4.NetDevs["foo"].Tags["netfoo"] = new(Entry) - test_node4.NetDevs["foo"].Tags["netfoo"].Set("netbaar") - nodeConf.GetFrom(test_node4) + test_empty.NetDevs["foo"].Tags = make(map[string]*Entry) + test_empty.NetDevs["foo"].Tags["netfoo"] = new(Entry) + test_empty.NetDevs["foo"].Tags["netfoo"].Set("netbaar") + nodeConf.GetFrom(test_empty) nodeConf.Flatten() wanted = `network devices: foo: @@ -340,9 +348,9 @@ ipmi: {} t.Errorf("Couldn't set nettag: '%s' got: '%s'", wanted, string(ymlByte)) } - delete(test_node4.NetDevs, "foo") + delete(test_empty.NetDevs, "foo") nodeConf = NewConf() - nodeConf.GetFrom(test_node4) + nodeConf.GetFrom(test_empty) nodeConf.Flatten() ymlByte, _ = yaml.Marshal(nodeConf) wanted = `{} diff --git a/internal/pkg/node/transformers.go b/internal/pkg/node/transformers.go index feabcc7c..61ff21d0 100644 --- a/internal/pkg/node/transformers.go +++ b/internal/pkg/node/transformers.go @@ -74,8 +74,11 @@ func recursiveGetter( if !targetValue.Elem().Field(i).MapIndex(sourceIter.Key()).IsValid() { // Only write entries for which have real values. This matters for // tags, as empty map elements can be created without this check - if ((sourceIter.Value().Interface()).(*Entry)).GotReal() { - str := getter((sourceIter.Value().Interface()).(*Entry)) + // The alternative was following check: + // if ((sourceIter.Value().Interface()).(*Entry)).GotReal() { + // but this one failed for tags in templates + str := getter((sourceIter.Value().Interface()).(*Entry)) + if str != "" { targetValue.Elem().Field(i).SetMapIndex(sourceIter.Key(), reflect.ValueOf(str)) } } diff --git a/internal/pkg/testenv/testenv.go b/internal/pkg/testenv/testenv.go index 15ea117a..7303d326 100644 --- a/internal/pkg/testenv/testenv.go +++ b/internal/pkg/testenv/testenv.go @@ -29,6 +29,19 @@ type TestEnv struct { BaseDir string } +const Sysconfdir = "etc" +const Bindir = "bin" +const Datadir = "share" +const Localstatedir = "var/local" +const Srvdir = "srv" +const Tftpdir = "srv/tftp" +const Firewallddir = "usr/lib/firewalld/services" +const Systemddir = "usr/lib/systemd/system" +const WWOverlaydir = "var/local/warewulf/overlays" +const WWChrootdir = "var/local/warewulf/chroots" +const WWProvisiondir = "srv/warewulf" +const WWClientdir = "warewulf" + // New creates a test environment in a temporary directory and configures // Warewulf to use it. // @@ -44,27 +57,27 @@ func New(t *testing.T) (env *TestEnv) { assert.NoError(t, err) env.BaseDir = tmpDir - env.WriteFile(t, "etc/warewulf/nodes.conf", initNodesConf) - env.WriteFile(t, "etc/warewulf/warewulf.conf", initWarewulfConf) - env.WriteFile(t, "share/warewulf/defaults.conf", initDefaultsConf) + env.WriteFile(t, path.Join(Sysconfdir, "warewulf/nodes.conf"), initNodesConf) + env.WriteFile(t, path.Join(Sysconfdir, "warewulf/warewulf.conf"), initWarewulfConf) + env.WriteFile(t, path.Join(Datadir, "warewulf/defaults.conf"), initDefaultsConf) // re-read warewulf.conf conf := config.New() - err = conf.Read(env.GetPath("etc/warewulf/warewulf.conf")) + err = conf.Read(env.GetPath(path.Join(Sysconfdir, "warewulf/warewulf.conf"))) assert.NoError(t, err) - conf.Paths.Sysconfdir = env.GetPath("etc") - conf.Paths.Bindir = env.GetPath("bin") - conf.Paths.Datadir = env.GetPath("share") - conf.Paths.Localstatedir = env.GetPath("var/local") - conf.Paths.Srvdir = env.GetPath("srv") - conf.Paths.Tftpdir = env.GetPath("srv/tftp") - conf.Paths.Firewallddir = env.GetPath("usr/lib/firewalld/services") - conf.Paths.Systemddir = env.GetPath("usr/lib/systemd/system") - conf.Paths.WWOverlaydir = env.GetPath(path.Join(conf.Paths.Localstatedir, "warewulfoverlays")) - conf.Paths.WWChrootdir = env.GetPath(path.Join(conf.Paths.Localstatedir, "warewulf/chroots")) - conf.Paths.WWProvisiondir = env.GetPath(path.Join(conf.Paths.Srvdir, "warewulf")) - conf.Paths.WWClientdir = env.GetPath("warewulf") + conf.Paths.Sysconfdir = env.GetPath(Sysconfdir) + conf.Paths.Bindir = env.GetPath(Bindir) + conf.Paths.Datadir = env.GetPath(Datadir) + conf.Paths.Localstatedir = env.GetPath(Localstatedir) + conf.Paths.Srvdir = env.GetPath(Srvdir) + conf.Paths.Tftpdir = env.GetPath(Tftpdir) + conf.Paths.Firewallddir = env.GetPath(Firewallddir) + conf.Paths.Systemddir = env.GetPath(Systemddir) + conf.Paths.WWOverlaydir = env.GetPath(WWOverlaydir) + conf.Paths.WWChrootdir = env.GetPath(WWChrootdir) + conf.Paths.WWProvisiondir = env.GetPath(WWProvisiondir) + conf.Paths.WWClientdir = env.GetPath(WWClientdir) for _, confPath := range []string{ conf.Paths.Sysconfdir, @@ -84,7 +97,7 @@ func New(t *testing.T) (env *TestEnv) { } // node.init() has already run, so set the config path again - node.ConfigFile = env.GetPath("etc/warewulf/nodes.conf") + node.ConfigFile = env.GetPath(path.Join(Sysconfdir, "warewulf/nodes.conf")) return }