From 57d28c5bc9ec2685cd8751de9d8bf95eaf6a65fa Mon Sep 17 00:00:00 2001 From: Jonathon Anderson Date: Sun, 19 Jan 2025 04:26:01 -0700 Subject: [PATCH] Expand nodes when rendering overlay templates Before #1628, all default constructors for nodes and profiles attempted to construct objects with pre-populated empty fields (e.g., for pointers to member structs and for collections that default to nil). Changes to MergeNode in #1629 changed this behavior intentionally to simplify the generation of minimal json; but this had the unintended side-effect of breaking existing templates that may now encounter a nil pointer, particularly when accessing member structs. Introduced here, the Expand() methods on Node and Profile are now responsible for populating such fields, and Node.Expand() is now called during template processing. Signed-off-by: Jonathon Anderson --- internal/pkg/node/methods.go | 39 ++++++++++++++----- internal/pkg/node/methods_test.go | 54 +++++++++++++++++++++++++++ internal/pkg/overlay/datastructure.go | 7 ++-- internal/pkg/overlay/overlay_test.go | 22 +++++++++++ overlays/debug/internal/debug_test.go | 10 +++++ 5 files changed, 119 insertions(+), 13 deletions(-) diff --git a/internal/pkg/node/methods.go b/internal/pkg/node/methods.go index 2483f825..b7687492 100644 --- a/internal/pkg/node/methods.go +++ b/internal/pkg/node/methods.go @@ -99,11 +99,7 @@ func NewProfile(id string) (profileconf Profile) { } func EmptyNode() (nodeconf Node) { - nodeconf.Ipmi = new(IpmiConf) - nodeconf.Ipmi.Tags = map[string]string{} - nodeconf.Kernel = new(KernelConf) - nodeconf.NetDevs = make(map[string]*NetDev) - nodeconf.Tags = map[string]string{} + nodeconf.Expand() return nodeconf } @@ -111,14 +107,37 @@ func EmptyNode() (nodeconf Node) { Creates a ProfileConf but doesn't add it to the database. */ func EmptyProfile() (profileconf Profile) { - profileconf.Ipmi = new(IpmiConf) - profileconf.Ipmi.Tags = map[string]string{} - profileconf.Kernel = new(KernelConf) - profileconf.NetDevs = make(map[string]*NetDev) - profileconf.Tags = map[string]string{} + profileconf.Expand() return profileconf } +func (nodeconf *Node) Expand() { + nodeconf.Profile.Expand() +} + +func (profile *Profile) Expand() { + if profile.Ipmi == nil { + profile.Ipmi = new(IpmiConf) + } + if profile.Ipmi.Tags == nil { + profile.Ipmi.Tags = make(map[string]string) + } + if profile.Kernel == nil { + profile.Kernel = new(KernelConf) + } + if profile.NetDevs == nil { + profile.NetDevs = make(map[string]*NetDev) + } + for i := range profile.NetDevs { + if profile.NetDevs[i].Tags == nil { + profile.NetDevs[i].Tags = make(map[string]string) + } + } + if profile.Tags == nil { + profile.Tags = make(map[string]string) + } +} + /* Flattens out a NodeConf, which means if there are no explicit values in *IpmiConf or *KernelConf, these pointer will set to nil. This will remove something like diff --git a/internal/pkg/node/methods_test.go b/internal/pkg/node/methods_test.go index 860e8257..675f9273 100644 --- a/internal/pkg/node/methods_test.go +++ b/internal/pkg/node/methods_test.go @@ -3,6 +3,8 @@ package node import ( "net" "testing" + + "github.com/stretchr/testify/assert" ) func Test_Empty(t *testing.T) { @@ -33,3 +35,55 @@ func Test_Empty(t *testing.T) { } }) } + +func Test_Node_Expand_Flatten(t *testing.T) { + node := new(Node) + + assert.Nil(t, node.Ipmi) + assert.Nil(t, node.Kernel) + assert.Nil(t, node.NetDevs) + assert.Nil(t, node.Tags) + + t.Run("test expand", func(t *testing.T) { + node.Expand() + assert.Equal(t, map[string]string{}, node.Tags) + assert.Equal(t, map[string]string{}, node.Ipmi.Tags) + assert.Equal(t, "", node.Kernel.Version) + assert.Equal(t, "", node.Kernel.Args) + assert.Equal(t, map[string]*NetDev{}, node.NetDevs) + }) + + t.Run("test flatten", func(t *testing.T) { + node.Flatten() + assert.Nil(t, node.Ipmi) + assert.Nil(t, node.Kernel) + assert.Nil(t, node.NetDevs) + assert.Nil(t, node.Tags) + }) +} + +func Test_Profile_Expand_Flatten(t *testing.T) { + profile := new(Profile) + + assert.Nil(t, profile.Ipmi) + assert.Nil(t, profile.Kernel) + assert.Nil(t, profile.NetDevs) + assert.Nil(t, profile.Tags) + + t.Run("test expand", func(t *testing.T) { + profile.Expand() + assert.Equal(t, map[string]string{}, profile.Tags) + assert.Equal(t, map[string]string{}, profile.Ipmi.Tags) + assert.Equal(t, "", profile.Kernel.Version) + assert.Equal(t, "", profile.Kernel.Args) + assert.Equal(t, map[string]*NetDev{}, profile.NetDevs) + }) + + t.Run("test flatten", func(t *testing.T) { + profile.Flatten() + assert.Nil(t, profile.Ipmi) + assert.Nil(t, profile.Kernel) + assert.Nil(t, profile.NetDevs) + assert.Nil(t, profile.Tags) + }) +} diff --git a/internal/pkg/overlay/datastructure.go b/internal/pkg/overlay/datastructure.go index 471a6899..bd783d03 100644 --- a/internal/pkg/overlay/datastructure.go +++ b/internal/pkg/overlay/datastructure.go @@ -53,9 +53,7 @@ func InitStruct(overlayName string, nodeData node.Node, allNodes []node.Node) (T tstruct.BuildHost = hostname controller := warewulfconf.Get() tstruct.ThisNode = &nodeData - if tstruct.ThisNode.Kernel == nil { - tstruct.ThisNode.Kernel = new(node.KernelConf) - } + tstruct.ThisNode.Expand() if tstruct.ThisNode.Kernel.Version == "" { if kernel_ := kernel.FromNode(tstruct.ThisNode); kernel_ != nil { tstruct.ThisNode.Kernel.Version = kernel_.Version() @@ -77,6 +75,9 @@ func InitStruct(overlayName string, nodeData node.Node, allNodes []node.Node) (T tstruct.Container = nodeData.ContainerName // Backwards compatibility for templates using "Keys" tstruct.AllNodes = allNodes + for i := range tstruct.AllNodes { + tstruct.AllNodes[i].Expand() + } dt := time.Now() tstruct.BuildTime = dt.Format("01-02-2006 15:04:05 MST") tstruct.BuildTimeUnix = strconv.FormatInt(dt.Unix(), 10) diff --git a/internal/pkg/overlay/overlay_test.go b/internal/pkg/overlay/overlay_test.go index 845ba11f..4843a3be 100644 --- a/internal/pkg/overlay/overlay_test.go +++ b/internal/pkg/overlay/overlay_test.go @@ -210,6 +210,28 @@ T3 "test-link": "/test-target", }, }, + "expansion of nodes": { + overlays: []string{"o1"}, + overlayFiles: map[string]string{ + "/var/lib/warewulf/overlays/o1/rootfs/node.txt.ww": ` +IPMI user:{{ .Ipmi.UserName}} +Kernel Version:{{.Kernel.Version}} +Kernel Args:{{.Kernel.Args}} +NetDevs:{{.NetDevs}} +Tags:{{.Tags}} +`, + }, + outputDir: "/image", + outputFiles: map[string]string{ + "node.txt": ` +IPMI user: +Kernel Version: +Kernel Args: +NetDevs:map[] +Tags:map[] +`, + }, + }, } for name, tt := range tests { diff --git a/overlays/debug/internal/debug_test.go b/overlays/debug/internal/debug_test.go index 16107854..ab38627f 100644 --- a/overlays/debug/internal/debug_test.go +++ b/overlays/debug/internal/debug_test.go @@ -259,6 +259,16 @@ data from other structures. - Version: - Args: quiet crashkernel=no vga=791 net.naming-scheme=v238 + - Ipmi: + - Ipaddr: + - Netmask: + - Port: + - Gateway: + - UserName: + - Password: + - Interface: + - Write: + - Tags: - NetDevs[default]: - Type: - OnBoot: true