Merge pull request #102 from Cray-HPE/provider-metadata-refactor

Some refactoring on how provider metadata is handled

cmd/cabinet/add_cabinet.go

@@ -67,7 +67,7 @@ func addCabinet(cmd *cobra.Command, args []string) error {
 		log.Info().Msgf("Querying inventory to suggest cabinet number and VLAN ID")
 		// set the vars to the recommendations
 		cabinetNumber = recommendations.LocationOrdinal
-		vlanId = recommendations.ProviderMetadata[csm.ProviderPropertyVlanId].(int)
+		vlanId = recommendations.ProviderMetadata[csm.ProviderMetadataVlanId].(int)
 		log.Debug().Msgf("Provider recommendations: %+v", recommendations)
 		log.Info().Msgf("Suggested cabinet number: %d", cabinetNumber)
 		log.Info().Msgf("Suggested VLAN ID: %d", vlanId)
@@ -97,7 +97,7 @@ func addCabinet(cmd *cobra.Command, args []string) error {
 	// Right now the build metadata function in the CSM provider will
 	// unset options if nil is passed in.
 	cabinetMetadata := map[string]interface{}{
-		csm.ProviderPropertyVlanId: vlanId,
+		csm.ProviderMetadataVlanId: vlanId,
 	}
 
 	// Add the cabinet to the inventory using domain methods

cmd/cabinet/list_cabinet.go

@@ -38,8 +38,8 @@ import (
 	"github.com/Cray-HPE/cani/internal/inventory"
 	"github.com/Cray-HPE/cani/internal/provider/csm"
 	"github.com/Cray-HPE/cani/pkg/hardwaretypes"
+	"github.com/Cray-HPE/cani/pkg/pointers"
 	"github.com/google/uuid"
-	"github.com/mitchellh/mapstructure"
 	"github.com/spf13/cobra"
 )
 
@@ -126,17 +126,26 @@ func listCabinet(cmd *cobra.Command, args []string) error {
 			return filtered[keys[i]].LocationPath.String() < filtered[keys[j]].LocationPath.String()
 		})
 
-		properties := csm.CabinetMetadata{}
 		for _, hw := range keys {
-			if _, exists := filtered[hw].ProviderProperties[string(inventory.CSMProvider)]; exists {
-				if err := mapstructure.Decode(filtered[hw].ProviderProperties["csm"], &properties); err != nil {
+			// Start with an empty cabinet metadata struct, just in case if this cabinet doesn't have any
+			// metadata set
+			cabinetMetadata := csm.CabinetMetadata{}
+
+			if _, exists := filtered[hw].ProviderMetadata[inventory.CSMProvider]; exists {
+				csmMetadata, err := csm.DecodeProviderMetadata(filtered[hw])
+				if err != nil {
 					return err
 				}
+
+				if csmMetadata.Cabinet != nil {
+					cabinetMetadata = *csmMetadata.Cabinet
+				}
 			}
+
 			fmt.Fprintf(w, "%s\t%s\t%v\t%s\n",
 				filtered[hw].ID.String(),
 				filtered[hw].DeviceTypeSlug,
-				*properties.HMNVlan,
+				pointers.IntPtrToStr(cabinetMetadata.HMNVlan),
 				filtered[hw].LocationPath.String())
 		}
 

cmd/node/list_node.go

@@ -39,7 +39,6 @@ import (
 	"github.com/Cray-HPE/cani/internal/provider/csm"
 	"github.com/Cray-HPE/cani/pkg/hardwaretypes"
 	"github.com/google/uuid"
-	"github.com/mitchellh/mapstructure"
 	"github.com/spf13/cobra"
 )
 
@@ -126,15 +125,25 @@ func listNode(cmd *cobra.Command, args []string) error {
 			return filtered[keys[i]].LocationPath.String() < filtered[keys[j]].LocationPath.String()
 		})
 
-		properties := csm.NodeMetadata{}
 		for _, hw := range keys {
-			if _, exists := filtered[hw].ProviderProperties[string(inventory.CSMProvider)]; exists {
-				if err := mapstructure.Decode(filtered[hw].ProviderProperties["csm"], &properties); err != nil {
+			// Start with an empty Node metadata struct, just in case if this node doesn't have any
+			// metadata set
+			var nodeMetadata csm.NodeMetadata
+
+			// If metadata exists decode it
+			if _, exists := filtered[hw].ProviderMetadata[inventory.CSMProvider]; exists {
+				csmMetadata, err := csm.DecodeProviderMetadata(filtered[hw])
+				if err != nil {
 					return err
 				}
+
+				if csmMetadata.Node != nil {
+					nodeMetadata = *csmMetadata.Node
+				}
 			}
+
 			// convert properties to strings and set nil values for easy printing
-			pp := properties.Pretty()
+			pp := nodeMetadata.Pretty()
 
 			fmt.Fprintf(w, "%s\t%s\t%v\t%v\t%v\t%v\t%v\n",
 				filtered[hw].ID.String(),

cmd/node/update_node.go

@@ -32,6 +32,7 @@ import (
 	root "github.com/Cray-HPE/cani/cmd"
 	"github.com/Cray-HPE/cani/internal/domain"
 	"github.com/Cray-HPE/cani/internal/provider"
+	"github.com/Cray-HPE/cani/internal/provider/csm"
 	"github.com/rs/zerolog/log"
 	"github.com/spf13/cobra"
 )
@@ -60,16 +61,16 @@ func updateNode(cmd *cobra.Command, args []string) error {
 	// unset options if nil is passed in.
 	nodeMeta := map[string]interface{}{}
 	if cmd.Flags().Changed("role") {
-		nodeMeta["role"] = role
+		nodeMeta[csm.ProviderMetadataRole] = role
 	}
 	if cmd.Flags().Changed("subrole") {
-		nodeMeta["subrole"] = subrole
+		nodeMeta[csm.ProviderMetadataSubRole] = subrole
 	}
 	if cmd.Flags().Changed("alias") {
-		nodeMeta["alias"] = alias
+		nodeMeta[csm.ProviderMetadataAlias] = alias
 	}
 	if cmd.Flags().Changed("alias") {
-		nodeMeta["nid"] = nid
+		nodeMeta[csm.ProviderMetadataNID] = nid
 	}
 
 	// Remove the node from the inventory using domain methods

internal/domain/node.go

@@ -58,7 +58,7 @@ func (d *Domain) UpdateNode(ctx context.Context, cabinet, chassis, slot, bmc, no
 		return AddHardwareResult{}, err
 	}
 
-	log.Debug().Any("metadata", hw.ProviderProperties).Msg("Provider Properties")
+	log.Debug().Any("metadata", hw.ProviderMetadata).Msg("Provider Properties")
 
 	// Push it back into the data store
 	if err := d.datastore.Update(&hw); err != nil {

internal/inventory/model.go

@@ -88,19 +88,16 @@ func (i *Inventory) FilterHardwareByTypeStatus(status HardwareStatus, types ...h
 // Hardware is the smallest unit of inventory
 // It has all the potential fields that hardware can have
 type Hardware struct {
-	ID                 uuid.UUID
-	Name               string                     `json:"Name,omitempty" yaml:"Name,omitempty" default:"" usage:"Friendly name"`
-	Type               hardwaretypes.HardwareType `json:"Type,omitempty" yaml:"Type,omitempty" default:"" usage:"Type"`
-	DeviceTypeSlug     string                     `json:"DeviceTypeSlug,omitempty" yaml:"DeviceTypeSlug,omitempty" default:"" usage:"Hardware Type Library Device slug"`
-	Vendor             string                     `json:"Vendor,omitempty" yaml:"Vendor,omitempty" default:"" usage:"Vendor"`
-	Architecture       string                     `json:"Architecture,omitempty" yaml:"Architecture,omitempty" default:"" usage:"Architecture"`
-	Model              string                     `json:"Model,omitempty" yaml:"Model,omitempty" default:"" usage:"Model"`
-	Status             HardwareStatus             `json:"Status,omitempty" yaml:"Status,omitempty" default:"Staged" usage:"Hardware can be [staged, provisioned, decomissioned]"`
-	Properties         map[string]interface{}     `json:"Properties,omitempty" yaml:"Properties,omitempty" default:"" usage:"Properties"`
-	Role               string                     `json:"Role,omitempty" yaml:"Role,omitempty" default:"" usage:"Role"`
-	SubRole            string                     `json:"SubRole,omitempty" yaml:"SubRole,omitempty" default:"" usage:"SubRole"`
-	Alias              string                     `json:"Alias,omitempty" yaml:"Alias,omitempty" default:"" usage:"Alias"`
-	ProviderProperties map[string]interface{}     `json:"ProviderProperties,omitempty" yaml:"ProviderProperties,omitempty" default:"" usage:"ProviderProperties"`
+	ID               uuid.UUID
+	Name             string                           `json:"Name,omitempty" yaml:"Name,omitempty" default:"" usage:"Friendly name"`
+	Type             hardwaretypes.HardwareType       `json:"Type,omitempty" yaml:"Type,omitempty" default:"" usage:"Type"`
+	DeviceTypeSlug   string                           `json:"DeviceTypeSlug,omitempty" yaml:"DeviceTypeSlug,omitempty" default:"" usage:"Hardware Type Library Device slug"`
+	Vendor           string                           `json:"Vendor,omitempty" yaml:"Vendor,omitempty" default:"" usage:"Vendor"`
+	Architecture     string                           `json:"Architecture,omitempty" yaml:"Architecture,omitempty" default:"" usage:"Architecture"`
+	Model            string                           `json:"Model,omitempty" yaml:"Model,omitempty" default:"" usage:"Model"`
+	Status           HardwareStatus                   `json:"Status,omitempty" yaml:"Status,omitempty" default:"Staged" usage:"Hardware can be [staged, provisioned, decomissioned]"`
+	Properties       map[string]interface{}           `json:"Properties,omitempty" yaml:"Properties,omitempty" default:"" usage:"Properties"`
+	ProviderMetadata map[Provider]ProviderMetadataRaw `json:"ProviderMetadata,omitempty" yaml:"ProviderMetadata,omitempty" default:"" usage:"ProviderMetadata"`
 
 	Parent uuid.UUID `json:"Parent,omitempty" yaml:"Parent,omitempty" default:"00000000-0000-0000-0000-000000000000" usage:"Parent hardware"`
 	// The following are derived from Parent
@@ -110,6 +107,16 @@ type Hardware struct {
 	LocationOrdinal *int
 }
 
+func (hardware *Hardware) SetProviderMetadata(provider Provider, metadata map[string]interface{}) {
+	// Initialize ProviderMetadata map if nil
+	if hardware.ProviderMetadata == nil {
+		hardware.ProviderMetadata = map[Provider]ProviderMetadataRaw{}
+	}
+
+	// Set provider metadata
+	hardware.ProviderMetadata[provider] = metadata
+}
+
 func NewHardwareFromBuildOut(hardwareBuildOut hardwaretypes.HardwareBuildOut, status HardwareStatus) Hardware {
 	locationOrdinal := hardwareBuildOut.OrdinalPath[len(hardwareBuildOut.OrdinalPath)-1]
 
@@ -149,6 +156,9 @@ const (
 	CSMProvider           = Provider("csm")
 )
 
+// ProviderMetadataRaw stores the metadata from a provider in a generic map.
+type ProviderMetadataRaw map[string]interface{}
+
 type LocationToken struct {
 	HardwareType hardwaretypes.HardwareType
 	Ordinal      int

internal/provider/csm/csv.go

@@ -38,10 +38,18 @@ import (
 func (csm *CSM) GetFields(hw *inventory.Hardware, fieldNames []string) (values []string, err error) {
 	values = make([]string, len(fieldNames))
 
-	rawCsmProps := hw.ProviderProperties["csm"]
-	csmProps, ok := rawCsmProps.(map[string]interface{})
-	if !ok {
-		csmProps = make(map[string]interface{})
+	nodeProps := make(map[string]interface{})
+	cabinetProps := make(map[string]interface{})
+	if csmProps, ok := hw.ProviderMetadata[inventory.CSMProvider]; ok {
+		nodePropsRaw, ok := csmProps["Node"]
+		if ok {
+			nodeProps = nodePropsRaw.(map[string]interface{})
+		}
+
+		cabinetPropsRaw, ok := csmProps["Cabinet"]
+		if ok {
+			cabinetProps = cabinetPropsRaw.(map[string]interface{})
+		}
 	}
 
 	for i, name := range fieldNames {
@@ -59,15 +67,15 @@ func (csm *CSM) GetFields(hw *inventory.Hardware, fieldNames []string) (values [
 		case "Status":
 			values[i] = fmt.Sprintf("%v", hw.Status)
 		case "Vlan":
-			values[i] = toString(csmProps["HMNVlan"])
+			values[i] = toString(cabinetProps["HMNVlan"])
 		case "Role":
-			values[i] = toString(csmProps["Role"])
+			values[i] = toString(nodeProps["Role"])
 		case "SubRole":
-			values[i] = toString(csmProps["SubRole"])
+			values[i] = toString(nodeProps["SubRole"])
 		case "Alias":
-			values[i] = getStringFromArray(csmProps["Alias"], 0)
+			values[i] = getStringFromArray(nodeProps["Alias"], 0)
 		case "Nid":
-			values[i] = toString(csmProps["Nid"])
+			values[i] = toString(nodeProps["Nid"])
 		default:
 			// This case should never be hit.
 			// The call to normalize should return an error for unknown headers
@@ -80,36 +88,36 @@ func (csm *CSM) GetFields(hw *inventory.Hardware, fieldNames []string) (values [
 }
 
 func (csm *CSM) SetFields(hw *inventory.Hardware, values map[string]string) (result provider.SetFieldsResult, err error) {
-	csmHardware, err := ToCsmHardware(hw)
+	csmMetadata, err := DecodeProviderMetadata(*hw)
 	if err != nil {
 		return result, err
 	}
 
-	if csmHardware.CabinetMetadata == nil && csmHardware.NodeMetadata == nil {
+	if csmMetadata.Cabinet == nil && csmMetadata.Node == nil {
 		log.Debug().Msgf("Skipping %v of the type %v. It does not have writable properties", hw.ID, hw.Type)
 		return
 	}
 
-	if csmHardware.NodeMetadata != nil {
+	if csmMetadata.Node != nil {
 		for key, value := range values {
 			switch key {
 			case "Role":
-				modified := setRole(value, csmHardware.NodeMetadata)
+				modified := setRole(value, csmMetadata.Node)
 				if modified {
 					result.ModifiedFields = append(result.ModifiedFields, "Role")
 				}
 			case "SubRole":
-				modified := setSubRole(value, csmHardware.NodeMetadata)
+				modified := setSubRole(value, csmMetadata.Node)
 				if modified {
 					result.ModifiedFields = append(result.ModifiedFields, "SubRole")
 				}
 			case "Alias":
-				modified := setAlias(value, csmHardware.NodeMetadata)
+				modified := setAlias(value, csmMetadata.Node)
 				if modified {
 					result.ModifiedFields = append(result.ModifiedFields, "Alias")
 				}
 			case "Nid":
-				modified, err := setNid(value, csmHardware.NodeMetadata)
+				modified, err := setNid(value, csmMetadata.Node)
 				if err != nil {
 					return result, err
 				}
@@ -118,11 +126,18 @@ func (csm *CSM) SetFields(hw *inventory.Hardware, values map[string]string) (res
 				}
 			}
 		}
-	} else if csmHardware.CabinetMetadata != nil {
+		if len(result.ModifiedFields) > 0 {
+			metadataRaw, err := EncodeProviderMetadata(Metadata{Node: csmMetadata.Node})
+			if err != nil {
+				return result, err
+			}
+			hw.SetProviderMetadata(inventory.CSMProvider, metadataRaw)
+		}
+	} else if csmMetadata.Cabinet != nil {
 		for key, value := range values {
 			switch key {
 			case "Vlan":
-				modified, err := setVlan(value, csmHardware.CabinetMetadata)
+				modified, err := setVlan(value, csmMetadata.Cabinet)
 				if err != nil {
 					return result, err
 				}
@@ -131,6 +146,13 @@ func (csm *CSM) SetFields(hw *inventory.Hardware, values map[string]string) (res
 				}
 			}
 		}
+		if len(result.ModifiedFields) > 0 {
+			metadataRaw, err := EncodeProviderMetadata(Metadata{Cabinet: csmMetadata.Cabinet})
+			if err != nil {
+				return result, err
+			}
+			hw.SetProviderMetadata(inventory.CSMProvider, metadataRaw)
+		}
 	}
 
 	return

internal/provider/csm/import.go

@@ -41,6 +41,7 @@ import (
 	"github.com/Cray-HPE/cani/internal/provider/csm/sls"
 	"github.com/Cray-HPE/cani/pkg/hardwaretypes"
 	hsm_client "github.com/Cray-HPE/cani/pkg/hsm-client"
+	"github.com/Cray-HPE/cani/pkg/pointers"
 	sls_client "github.com/Cray-HPE/cani/pkg/sls-client"
 	"github.com/Cray-HPE/hms-xname/xnames"
 	"github.com/Cray-HPE/hms-xname/xnametypes"
@@ -276,18 +277,22 @@ func (csm *CSM) Import(ctx context.Context, datastore inventory.Datastore) error
 			// Set cabinet metadata
 			cabinetMetadata := CabinetMetadata{}
 			if vlan, exists := cabinetHMNVlans[slsCabinet.Xname]; exists {
-				cabinetMetadata.HMNVlan = IntPtr(vlan)
+				cabinetMetadata.HMNVlan = pointers.IntPtr(vlan)
 			}
 
 			cCabinet, err = tempDatastore.GetAtLocation(locationPath)
 			if err != nil {
 				return errors.Join(fmt.Errorf("failed to query datastore for %s", locationPath), err)
 			}
 
-			cCabinet.ProviderProperties = map[string]interface{}{
-				"csm": cabinetMetadata,
+			// Encode cabinet metadata
+			metadataRaw, err := EncodeProviderMetadata(Metadata{Cabinet: &cabinetMetadata})
+			if err != nil {
+				return fmt.Errorf("failed to encode provider metadata for hardware (%s)", cCabinet.ID)
 			}
+			cCabinet.SetProviderMetadata(inventory.CSMProvider, metadataRaw)
 
+			// Push cabinet in datastore.
 			if err := tempDatastore.Update(&cCabinet); err != nil {
 				return fmt.Errorf("failed to update hardware (%s) in memory datastore", cCabinet.ID)
 			}
@@ -590,15 +595,15 @@ func (csm *CSM) Import(ctx context.Context, datastore inventory.Datastore) error
 
 		nodeMetadata := NodeMetadata{}
 		if slsNodeEP.Role != "" {
-			nodeMetadata.Role = StringPtr(slsNodeEP.Role)
+			nodeMetadata.Role = pointers.StringPtr(slsNodeEP.Role)
 		}
 
 		if slsNodeEP.SubRole != "" {
-			nodeMetadata.Role = StringPtr(slsNodeEP.SubRole)
+			nodeMetadata.Role = pointers.StringPtr(slsNodeEP.SubRole)
 		}
 
 		if slsNodeEP.NID != 0 {
-			nodeMetadata.Nid = IntPtr(int(slsNodeEP.NID))
+			nodeMetadata.Nid = pointers.IntPtr(int(slsNodeEP.NID))
 		}
 
 		if len(slsNodeEP.Aliases) != 0 {
@@ -644,11 +649,12 @@ func (csm *CSM) Import(ctx context.Context, datastore inventory.Datastore) error
 			return errors.Join(fmt.Errorf("failed to query datastore for %s", nodeLocationPath), err)
 		}
 
-		// Initialize the properties map if not done already
-		if cNode.ProviderProperties == nil {
-			cNode.ProviderProperties = map[string]interface{}{}
+		// Set the node metadata
+		metadataRaw, err := EncodeProviderMetadata(Metadata{Node: &nodeMetadata})
+		if err != nil {
+			return fmt.Errorf("failed to encode provider metadata for hardware (%s)", cNode.ID)
 		}
-		cNode.ProviderProperties["csm"] = nodeMetadata
+		cNode.SetProviderMetadata(inventory.CSMProvider, metadataRaw)
 
 		// Push updates into the datastore
 		if err := tempDatastore.Update(&cNode); err != nil {