From 83af6021c0cf142ac0e75335d85a9dfce130806d Mon Sep 17 00:00:00 2001 From: Jonathon Anderson Date: Tue, 17 Dec 2024 01:34:19 -0700 Subject: [PATCH] Recast Profiles as a Profile field and support recursion Signed-off-by: Jonathon Anderson --- CHANGELOG.md | 1 + internal/app/wwctl/node/set/main_test.go | 2 + internal/app/wwctl/profile/list/main_test.go | 3 + internal/pkg/container/syncuids_test.go | 1 - internal/pkg/node/datastructure.go | 2 +- internal/pkg/node/fields_test.go | 1 + internal/pkg/node/mergo.go | 94 +++++++- internal/pkg/node/mergo_test.go | 225 +++++++++++++++++++ internal/pkg/upgrade/node.go | 4 +- internal/pkg/upgrade/node_test.go | 20 ++ userdocs/contents/profiles.rst | 10 + 11 files changed, 356 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f088b583..ad3e53e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Add possibility to define a softlink target with an overlay template - Support defining a symlink with an overlay template. #1303 - New "localtime" overlay to define the system time zone. #1303 +- Add support for nested profiles. #1572, #1598 ### Changed diff --git a/internal/app/wwctl/node/set/main_test.go b/internal/app/wwctl/node/set/main_test.go index 269a95eb..394257d2 100644 --- a/internal/app/wwctl/node/set/main_test.go +++ b/internal/app/wwctl/node/set/main_test.go @@ -522,6 +522,8 @@ nodes: n01: profiles: - default + disks: + /dev/vda: {} filesystems: /dev/disk/by-partlabel/var: format: btrfs diff --git a/internal/app/wwctl/profile/list/main_test.go b/internal/app/wwctl/profile/list/main_test.go index 9c3f19b1..8286d1d9 100644 --- a/internal/app/wwctl/profile/list/main_test.go +++ b/internal/app/wwctl/profile/list/main_test.go @@ -154,6 +154,7 @@ nodes: output: ` { "default": { + "Profiles": null, "Comment": "", "ClusterName": "", "ContainerName": "", @@ -204,6 +205,7 @@ nodes: output: ` { "default": { + "Profiles": null, "Comment": "", "ClusterName": "", "ContainerName": "", @@ -221,6 +223,7 @@ nodes: "FileSystems": null }, "test": { + "Profiles": null, "Comment": "", "ClusterName": "", "ContainerName": "", diff --git a/internal/pkg/container/syncuids_test.go b/internal/pkg/container/syncuids_test.go index efd3c6f3..75a6401f 100644 --- a/internal/pkg/container/syncuids_test.go +++ b/internal/pkg/container/syncuids_test.go @@ -282,6 +282,5 @@ func Test_network_passwd(t *testing.T) { db := make(syncDB) err := db.readFromHost(hostFileName) assert.NotContains(t, buf.String(), "parse error") - assert.Contains(t, buf.String(), "Ignoring line") assert.NoError(t, err) } diff --git a/internal/pkg/node/datastructure.go b/internal/pkg/node/datastructure.go index e3a08bb6..c6789203 100644 --- a/internal/pkg/node/datastructure.go +++ b/internal/pkg/node/datastructure.go @@ -28,7 +28,6 @@ type Node struct { // exported values Discoverable wwtype.WWbool `yaml:"discoverable,omitempty" lopt:"discoverable" sopt:"e" comment:"Make discoverable in given network (true/false)"` AssetKey string `yaml:"asset key,omitempty" lopt:"asset" comment:"Set the node's Asset tag (key)"` - Profiles []string `yaml:"profiles,omitempty" lopt:"profile" sopt:"P" comment:"Set the node's profile members (comma separated)"` Profile `yaml:"-,inline"` // include all values set in the profile, but inline them in yaml output if these are part of Node } @@ -38,6 +37,7 @@ Holds the data which can be set for profiles and nodes. type Profile struct { id string // exported values + Profiles []string `yaml:"profiles,omitempty" lopt:"profile" sopt:"P" comment:"Set the node's profile members (comma separated)"` Comment string `yaml:"comment,omitempty" lopt:"comment" comment:"Set arbitrary string comment"` ClusterName string `yaml:"cluster name,omitempty" lopt:"cluster" sopt:"c" comment:"Set cluster group"` ContainerName string `yaml:"container name,omitempty" lopt:"container" sopt:"C" comment:"Set container name"` diff --git a/internal/pkg/node/fields_test.go b/internal/pkg/node/fields_test.go index f9d6ac8b..bd7f33b1 100644 --- a/internal/pkg/node/fields_test.go +++ b/internal/pkg/node/fields_test.go @@ -158,6 +158,7 @@ func Test_listFields(t *testing.T) { }, }, fields: []string{ + "Profiles", "Comment", "ClusterName", "ContainerName", diff --git a/internal/pkg/node/mergo.go b/internal/pkg/node/mergo.go index 39902a10..1b6be75c 100644 --- a/internal/pkg/node/mergo.go +++ b/internal/pkg/node/mergo.go @@ -8,15 +8,26 @@ import ( "dario.cat/mergo" + "github.com/warewulf/warewulf/internal/pkg/util" "github.com/warewulf/warewulf/internal/pkg/wwlog" ) -func copyProfile(this Profile) (Profile, error) { +// copyProfile creates a deep copy of the given Profile object. +// It uses encoding/gob to serialize and deserialize the Profile, ensuring +// that all nested fields are copied. +// +// Parameters: +// - original: The Profile object to be copied. +// +// Returns: +// - A new Profile object that is a deep copy of the input. +// - An error if serialization or deserialization fails. +func copyProfile(original Profile) (Profile, error) { var buf bytes.Buffer enc := gob.NewEncoder(&buf) dec := gob.NewDecoder(&buf) profile := Profile{} - if err := enc.Encode(this); err != nil { + if err := enc.Encode(original); err != nil { return profile, err } else { if err := dec.Decode(&profile); err != nil { @@ -27,6 +38,75 @@ func copyProfile(this Profile) (Profile, error) { } } +// getNodeProfiles retrieves a list of profile IDs associated with a specific node ID. +// It retrives nested profiles and ensures the list is cleaned of duplicates +// and negations (denoted with a '~' prefix). +// +// Parameters: +// - id: The identifier of the node whose profiles are to be retrieved. +// +// Returns: +// - A slice of profile IDs associated with the given node ID. +func (config *NodesYaml) getNodeProfiles(id string) (profiles []string) { + if node, ok := config.Nodes[id]; ok { + for _, profileID := range node.Profiles { + profiles = cleanList(append(profiles, profileID)) + if !strings.HasPrefix(profileID, "~") { + profiles = config.appendProfileProfiles(profiles, profileID) + } + } + } + return cleanList(profiles) +} + +// appendProfileProfiles recursively appends profile IDs associated with a given profile ID +// to the provided list of profile IDs. It recursively processes nested profiles and ensures +// the list is cleaned of duplicates and negations (denoted with a '~' prefix). +// +// Profiles are only added if they do not already exist in the list. +// +// Parameters: +// - profiles: A slice of strings representing the current list of profiles by ID. +// - id: The identifier of the profile whose associated profiles are to be appended. +// +// Returns: +// - A slice of strings containing the updated list of profile IDs. +func (config *NodesYaml) appendProfileProfiles(profiles []string, id string) []string { + if profile, ok := config.NodeProfiles[id]; ok { + for _, subID := range profile.Profiles { + if !util.InSlice(profiles, subID) { + profiles = cleanList(append(profiles, subID)) + if !strings.HasPrefix(subID, "~") { + profiles = config.appendProfileProfiles(profiles, subID) + } + } + } + } + return profiles +} + +// MergeNode merges the configuration of a node identified by `id` with all the profiles +// associated with it, producing a fully composed `Node` and a `fieldMap` detailing the +// sources of various configuration fields. +// +// It works by: +// - Retrieving the base node configuration using `GetNodeOnly`. +// - Gathering all profile IDs associated with the node via `getNodeProfiles`. +// - For each profile: +// - Merging fields from a deep copy of each profile into the node, +// recording the origin of each configuration field (i.e., which profile provided it) +// in a `fieldMap` so that traceability is maintained. +// - Finally, merging the original node configuration back into the processed node, ensuring +// that any fields not set by the profiles are preserved, and updating the `fieldMap` +// accordingly. +// +// Parameters: +// - id: The identifier of the node to be merged with its profiles. +// +// Returns: +// - node: The resulting merged `Node` configuration. +// - fields: A `fieldMap` detailing the source(s) of each configuration field. +// - err: An error if any node or profile retrieval or merging operations fail. func (config *NodesYaml) MergeNode(id string) (node Node, fields fieldMap, err error) { node, err = config.GetNodeOnly(id) if err != nil { @@ -37,7 +117,7 @@ func (config *NodesYaml) MergeNode(id string) (node Node, fields fieldMap, err e fields = make(fieldMap) - for _, profileID := range cleanList(originalNode.Profiles) { + for _, profileID := range config.getNodeProfiles(id) { if profile, err := config.GetProfile(profileID); err != nil { wwlog.Warn("profile not found: %s", profileID) continue @@ -79,6 +159,14 @@ func (config *NodesYaml) MergeNode(id string) (node Node, fields fieldMap, err e } } + node.Profiles = originalNode.Profiles + if len(node.Profiles) > 0 { + fields.Set("Profiles", "", strings.Join(originalNode.Profiles, ",")) + fields["Profiles"].Source = "" + } else { + delete(fields, "Profiles") + } + node.id = id node.valid = true node.updatePrimaryNetDev() diff --git a/internal/pkg/node/mergo_test.go b/internal/pkg/node/mergo_test.go index 9c0e4a56..cf49c423 100644 --- a/internal/pkg/node/mergo_test.go +++ b/internal/pkg/node/mergo_test.go @@ -8,6 +8,188 @@ import ( "github.com/warewulf/warewulf/internal/pkg/testenv" ) +func Test_getNodeProfiles(t *testing.T) { + var tests = map[string]struct { + nodesConf string + node string + profiles []string + }{ + "no profiles": { + nodesConf: ` +nodes: + n1: {}`, + node: "n1", + profiles: nil, + }, + + "one profile": { + nodesConf: ` +nodes: + n1: + profiles: + - p1`, + node: "n1", + profiles: []string{"p1"}, + }, + "two profiles": { + nodesConf: ` +nodes: + n1: + profiles: + - p1 + - p2`, + node: "n1", + profiles: []string{"p1", "p2"}, + }, + "negated profiles": { + nodesConf: ` +nodes: + n1: + profiles: + - p1 + - p2 +nodeprofiles: + p2: + profiles: + - "~p1"`, + node: "n1", + profiles: []string{"p2"}, + }, + "negated missing profile": { + nodesConf: ` +nodes: + n1: + profiles: + - p1 + - p2 +nodeprofiles: + p2: + profiles: + - "~p3"`, + node: "n1", + profiles: []string{"p1", "p2"}, + }, + "single nested profile": { + nodesConf: ` +nodes: + n1: + profiles: + - p1 +nodeprofiles: + p1: + profiles: + - p2`, + node: "n1", + profiles: []string{"p1", "p2"}, + }, + "double nested profile": { + nodesConf: ` +nodes: + n1: + profiles: + - p1 +nodeprofiles: + p1: + profiles: + - p2 + p2: + profiles: + - p3`, + node: "n1", + profiles: []string{"p1", "p2", "p3"}, + }, + "negated nested profile": { + nodesConf: ` +nodes: + n1: + profiles: + - p1 +nodeprofiles: + p1: + profiles: + - p2 + p2: + profiles: + - "~p2" + - p3`, + node: "n1", + profiles: []string{"p1", "p3"}, + }, + "cicular nested profile": { + nodesConf: ` +nodes: + n1: + profiles: + - p1 +nodeprofiles: + p1: + profiles: + - p2 + p2: + profiles: + - p3 + p3: + profiles: + - p1`, + node: "n1", + profiles: []string{"p1", "p2", "p3"}, + }, + "cicular nested profile negation": { + nodesConf: ` +nodes: + n1: + profiles: + - p1 +nodeprofiles: + p1: + profiles: + - p2 + p2: + profiles: + - "~p1" + - p3 + p3: + profiles: + - p1`, + node: "n1", + profiles: []string{"p2", "p3", "p1"}, + }, + "repeated nested profile": { + nodesConf: ` +nodes: + n1: + profiles: + - pa1 + - pb1 +nodeprofiles: + pa1: + profiles: + - pa2 + pb1: + profiles: + - "~pa2" + - pb2 + pb2: + profiles: + - pa2`, + node: "n1", + profiles: []string{"pa1", "pb1", "pb2", "pa2"}, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + env := testenv.New(t) + defer env.RemoveAll(t) + env.WriteFile(t, "/etc/warewulf/nodes.conf", tt.nodesConf) + + registry, regErr := New() + assert.NoError(t, regErr) + assert.Equal(t, tt.profiles, registry.getNodeProfiles(tt.node)) + }) + } +} + func Test_MergeNode(t *testing.T) { var tests = map[string]struct { nodesConf string @@ -389,6 +571,49 @@ nodeprofiles: source: "p1,p2,n1", value: "po1,po2,po3,po4,no1,no2", }, + "node profiles": { + nodesConf: ` +nodes: + n1: + profiles: + - p1 + - p2`, + node: "n1", + field: "Profiles", + source: "", + value: "p1,p2", + }, + "nested profiles": { + nodesConf: ` +nodes: + n1: + profiles: + - p1 +nodeprofiles: + p1: + profiles: + - p2`, + node: "n1", + field: "Profiles", + source: "", + value: "p1", + }, + "negated profiles": { + nodesConf: ` +nodes: + n1: + profiles: + - p1 + - p2 +nodeprofiles: + p2: + profiles: + - "~p1"`, + node: "n1", + field: "Profiles", + source: "", + value: "p1,p2", + }, "node netdev tag": { nodesConf: ` nodes: diff --git a/internal/pkg/upgrade/node.go b/internal/pkg/upgrade/node.go index a8fdb7d1..7a940aa8 100644 --- a/internal/pkg/upgrade/node.go +++ b/internal/pkg/upgrade/node.go @@ -384,8 +384,8 @@ func (this *Profile) Upgrade(addDefaults bool, replaceOverlays bool) (upgraded * } } } - if this.Profiles != nil { - logIgnore("Profiles", this.Profiles, "invalid for profiles") + if upgraded.Profiles == nil { + upgraded.Profiles = append(upgraded.Profiles, this.Profiles...) } upgraded.Root = this.Root if this.RuntimeOverlay != nil { diff --git a/internal/pkg/upgrade/node_test.go b/internal/pkg/upgrade/node_test.go index f827bb4e..21ef5659 100644 --- a/internal/pkg/upgrade/node_test.go +++ b/internal/pkg/upgrade/node_test.go @@ -747,6 +747,26 @@ nodes: container name: mycontainer kernel: version: /boot/vmlinuz-1.2.3 +`, + }, + { + name: "Nested profiles", + addDefaults: false, + replaceOverlays: false, + legacyYaml: ` +nodeprofiles: + p1: + profiles: + - p2 + p2: {} +`, + upgradedYaml: ` +nodeprofiles: + p1: + profiles: + - p2 + p2: {} +nodes: {} `, }, } diff --git a/userdocs/contents/profiles.rst b/userdocs/contents/profiles.rst index 4dfe7f59..cbfda32b 100644 --- a/userdocs/contents/profiles.rst +++ b/userdocs/contents/profiles.rst @@ -10,6 +10,9 @@ customizations. For example, a few hundred nodes that are running a particular container or kernel, and another group of nodes that are running a different kernel or container. +Profiles may, themselves, reference other profiles, supporting complex +mixtures of profile configuration and negation. + Any node configuration attributes can be applied to a profile, but there are always going to be some node configurations which must be specific to a node, like a network HW/MAC address or an IP address. @@ -174,6 +177,13 @@ as can be seen here: n0000 Cluster test_profile cluster01 n0000 Profiles -- default,test_profile +Negating Profiles +================= + +Profiles may also be negated by later profiles. For example, ``p2,~p1`` +adds the profile ``p2`` to a node and removes a previously-applied ``p1`` +profile from a node. + How To Use Profiles Effectively ===============================