Skip to content

Format device model and keep updated in store #191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 19, 2024
Merged

Format device model and keep updated in store #191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
30928da
store/serverservice: fix up server vendor, model, serial updates
joelrebel Mar 19, 2024
f1be383
go: bump github.com/bmc-toolbox/common
joelrebel Mar 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ go 1.20

require (
github.com/bmc-toolbox/bmclib/v2 v2.0.1-0.20230825151635-6cf01686c513
github.com/bmc-toolbox/common v0.0.0-20230717121556-5eb9915a8a5a
github.com/bmc-toolbox/common v0.0.0-20231204194243-7bcbccab7116
github.com/bombsimon/logrusr/v2 v2.0.1
github.com/coreos/go-oidc v2.2.1+incompatible
github.com/equinix-labs/otel-init-go v0.0.7
Expand Down
10 changes: 2 additions & 8 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6r
github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs=
github.com/bmc-toolbox/bmclib/v2 v2.0.1-0.20230825151635-6cf01686c513 h1:FArNt6XfHzD55b67tw7UwMPL9Uz/6/bQ+weI1kPNbbU=
github.com/bmc-toolbox/bmclib/v2 v2.0.1-0.20230825151635-6cf01686c513/go.mod h1:a3Ra0ce/LV3wAj7AHuphlHNTx5Sg67iQqtLGr1zoqio=
github.com/bmc-toolbox/common v0.0.0-20230717121556-5eb9915a8a5a h1:SjtoU9dE3bYfYnPXODCunMztjoDgnE3DVJCPLBqwz6Q=
github.com/bmc-toolbox/common v0.0.0-20230717121556-5eb9915a8a5a/go.mod h1:SY//n1PJjZfbFbmAsB6GvEKbc7UXz3d30s3kWxfJQ/c=
github.com/bmc-toolbox/common v0.0.0-20231204194243-7bcbccab7116 h1:gqWn/cMjryKoUfITx2vRHrRHTvd9fQ+zKPwWsmMIrK4=
github.com/bmc-toolbox/common v0.0.0-20231204194243-7bcbccab7116/go.mod h1:SY//n1PJjZfbFbmAsB6GvEKbc7UXz3d30s3kWxfJQ/c=
github.com/bombsimon/logrusr/v2 v2.0.1 h1:1VgxVNQMCvjirZIYaT9JYn6sAVGVEcNtRE0y4mvaOAM=
github.com/bombsimon/logrusr/v2 v2.0.1/go.mod h1:ByVAX+vHdLGAfdroiMg6q0zgq2FODY2lc5YJvzmOJio=
github.com/bytedance/sonic v1.5.0/go.mod h1:ED5hyg4y6t3/9Ku1R6dU/4KyJ48DZ4jPhfY1O2AihPM=
Expand Down Expand Up @@ -517,12 +517,6 @@ github.com/mattn/go-sqlite3 v2.0.1+incompatible/go.mod h1:FPy6KqzDD04eiIsT53CuJW
github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 h1:jWpvCLoY8Z/e3VKvlsiIGKtc+UG6U5vzxaoagmhXfyg=
github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0/go.mod h1:QUyp042oQthUoa9bqDv0ER0wrtXnBruoNd7aNjkbP+k=
github.com/metal-toolbox/ironlib v0.2.15-0.20231222121932-c3b1c18fa034 h1:zpmLfbLxouW8vDqugG33kJO4CaMaJVVvkj5dSLd5cZA=
github.com/metal-toolbox/ironlib v0.2.15-0.20231222121932-c3b1c18fa034/go.mod h1:QRGRb//lV7LR2yDIe/zmXTFYGKhltQsRol05QlHzfBw=
github.com/metal-toolbox/ironlib v0.2.15-0.20231222142904-33805cc706da h1:f5H0nVwkhvRgBytQb1WTGpmLZ7CRSniekKmRO8sTnmM=
github.com/metal-toolbox/ironlib v0.2.15-0.20231222142904-33805cc706da/go.mod h1:QRGRb//lV7LR2yDIe/zmXTFYGKhltQsRol05QlHzfBw=
github.com/metal-toolbox/ironlib v0.2.15-0.20231222143939-19caf253762a h1:5GuMn4Gj39lJrZ3oDhdWUo2DfsbtTD2v/pm7uMVVwtw=
github.com/metal-toolbox/ironlib v0.2.15-0.20231222143939-19caf253762a/go.mod h1:QRGRb//lV7LR2yDIe/zmXTFYGKhltQsRol05QlHzfBw=
github.com/metal-toolbox/ironlib v0.2.15 h1:0uLmUnS7GgRZVwTOGJ+uyUlDEkSzoD+BYvZme9/C5tk=
github.com/metal-toolbox/ironlib v0.2.15/go.mod h1:QRGRb//lV7LR2yDIe/zmXTFYGKhltQsRol05QlHzfBw=
github.com/metal-toolbox/rivets v0.2.3-0.20231222134907-c139dbc19928 h1:j9suDVBwndsBQreWaqkS/3NDgPlRhHY3fk5uMTehSgg=
Expand Down
47 changes: 38 additions & 9 deletions internal/store/serverservice/attributes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"sort"

"github.com/bmc-toolbox/common"
"github.com/google/go-cmp/cmp"
"github.com/google/uuid"
"github.com/metal-toolbox/alloy/internal/helpers"
Expand Down Expand Up @@ -106,8 +107,9 @@ func (r *Store) deviceVendorData(asset *model.Asset) map[string]string {
m[serverSerialAttributeKey] = asset.Inventory.Serial
}

if asset.Inventory.Model != "" {
m[serverModelAttributeKey] = asset.Inventory.Model
invModelFormatted := common.FormatProductName(asset.Inventory.Model)
if invModelFormatted != "" {
m[serverModelAttributeKey] = invModelFormatted
}

if asset.Inventory.Vendor != "" {
Expand All @@ -119,21 +121,48 @@ func (r *Store) deviceVendorData(asset *model.Asset) map[string]string {
}

// returns a map with device vendor attributes when an update is required
//
// nolint:gocyclo // data verify is cyclomatic
func vendorDataUpdate(newData, currentData map[string]string) map[string]string {
const unknown = "unknown"
var changes bool

if currentData == nil {
return newData
}

var changes bool
newModelNumber := common.FormatProductName(newData[serverModelAttributeKey])
currentModelNumber := currentData[serverModelAttributeKey]

setValue := func(key string, newData, currentData map[string]string) {
const unknown = "unknown"
changeRequired := func(key string) bool {
newModelDefined := newModelNumber != unknown && newModelNumber != ""
currentModelUndefined := currentModelNumber == unknown || currentModelNumber == ""

// model number to be updated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we even querying things that will never change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the record, the reasons were discussed in another channel, to summarise,
theres a few exceptional cases like - when bmc-toolbox/common does not yet format the model number the model attribute in the store needs to be updated.

if key == serverModelAttributeKey {
if currentModelUndefined && newModelDefined {
return true
}

if currentData[key] == "" || currentData[key] == unknown {
if newData[key] != unknown {
changes = true
currentData[key] = newData[key]
if newModelDefined && currentModelNumber != newModelNumber {
newData[serverModelAttributeKey] = newModelNumber
return true
}

return false
}

// rest of the keys are updated when the new data is defined and current is not.
newKeyDefined := newData[key] != unknown && newData[key] != ""
currentKeyUndefined := currentData[key] == "" || currentData[key] == unknown
return newKeyDefined && currentKeyUndefined
}

setValue := func(key string, newData, currentData map[string]string) {
// value is empty or explicit check for model number
if changeRequired(key) {
changes = true
currentData[key] = newData[key]
}
}

Expand Down
154 changes: 154 additions & 0 deletions internal/store/serverservice/attributes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -966,3 +966,157 @@ func TestMetadataFilter(t *testing.T) {
got = mustFilterAssetMetadata(dirty)
require.Equal(t, json.RawMessage(exp), got, "dirty doesn't serialize properly")
}

func TestDeviceVendorData(t *testing.T) {
testcases := []struct {
name string
asset *model.Asset
expected map[string]string
}{
{
"default values",
&model.Asset{
ID: uuid.NewString(),
Inventory: &common.Device{
Common: common.Common{},
},
},
map[string]string{
serverSerialAttributeKey: "unknown",
serverVendorAttributeKey: "unknown",
serverModelAttributeKey: "unknown",
},
},
{
"expected values set",
&model.Asset{
ID: uuid.NewString(),
Inventory: &common.Device{
Common: common.Common{
Vendor: "foobar",
Model: "baz",
Serial: "00123",
},
},
},
map[string]string{
serverVendorAttributeKey: "foobar",
serverModelAttributeKey: "baz",
serverSerialAttributeKey: "00123",
},
},
{
"model attribute formatted",
&model.Asset{
ID: uuid.NewString(),
Inventory: &common.Device{
Common: common.Common{
Vendor: "foobar",
Model: "PIO-519C-MR-PH004",
Serial: "00123",
},
},
},
map[string]string{
serverVendorAttributeKey: "foobar",
serverModelAttributeKey: "x11sch-f",
serverSerialAttributeKey: "00123",
},
},
}

s := &Store{}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
got := s.deviceVendorData(tc.asset)
assert.Equal(t, tc.expected, got, tc.name)
})
}
}

func TestVendorDataUpdate(t *testing.T) {
tests := []struct {
name string
newData map[string]string
currentData map[string]string
expected map[string]string
}{
{
name: "updates when currentData is nil",
newData: map[string]string{"vendor": "foo", "model": "bar"},
currentData: nil,
expected: map[string]string{"vendor": "foo", "model": "bar"},
},
{
name: "updates model when current model is unknown",
newData: map[string]string{"model": "foo"},
currentData: map[string]string{"model": "unknown"},
expected: map[string]string{"model": "foo"},
},
{
name: "updates serial when current serial is unknown",
newData: map[string]string{"model": "foo", "serial": "123"},
currentData: map[string]string{"model": "foo", "serial": "unknown"},
expected: map[string]string{"model": "foo", "serial": "123"},
},
{
name: "updates serial when current serial is empty",
newData: map[string]string{"model": "foo", "serial": "123"},
currentData: map[string]string{"model": "foo", "serial": ""},
expected: map[string]string{"model": "foo", "serial": "123"},
},
{
name: "no update when new model matches current",
newData: map[string]string{"model": "r6515"},
currentData: map[string]string{"model": "r6515"},
expected: nil,
},
{
name: "no update when new vendor matches current",
newData: map[string]string{"vendor": "r6515"},
currentData: map[string]string{"vendor": "baz"},
expected: nil,
},
{
name: "no update when serial matches current",
newData: map[string]string{"model": "foo", "serial": "123"},
currentData: map[string]string{"model": "foo", "serial": "123"},
expected: nil,
},
{
name: "update model when current does not match new",
newData: map[string]string{"vendor": "bar", "model": "newfoo"},
currentData: map[string]string{"vendor": "baz", "model": "oldfoo"},
expected: map[string]string{"vendor": "baz", "model": "newfoo"},
},
{
name: "no update when new model is formatted and matches current",
newData: map[string]string{"vendor": "bar", "model": "PIO-519C-MR-PH004"},
currentData: map[string]string{"vendor": "baz", "model": "x11sch-f"},
expected: nil,
},
{
name: "update when new model is formatted and does not match current",
newData: map[string]string{"vendor": "bar", "model": "PIO-519C-MR-PH004"},
currentData: map[string]string{"vendor": "baz", "model": "PIO-519C-MR-PH004"},
expected: map[string]string{"vendor": "baz", "model": "x11sch-f"},
},
{
name: "ignore 'unknown' in new values",
newData: map[string]string{"model": "unknown", "vendor": "unknown"},
currentData: map[string]string{"model": "foo", "vendor": "bar"},
expected: nil,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := vendorDataUpdate(tc.newData, tc.currentData)
if tc.expected != nil {
assert.Equal(t, tc.expected, got, tc.name)
} else {
assert.Nil(t, got, tc.name)
}
})
}
}