Skip to content

fix(rest-api): keep ExpectedMachine BMC IPs synchronized #3701

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
chet merged 1 commit into from
Jul 19, 2026
+164 −10
Merged

fix(rest-api): keep ExpectedMachine BMC IPs synchronized #3701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 30 additions & 10 deletions rest-api/workflow/pkg/activity/expectedmachine/expectedmachine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ func (mei ManageExpectedMachine) UpdateExpectedMachinesInDB(ctx context.Context,
ChassisSerialNumber: reported.ChassisSerialNumber,
SkuID: reported.SkuID,
FallbackDpuSerialNumbers: reported.FallbackDpuSerialNumbers,
BmcIpAddress: reported.BmcIpAddress,
Labels: reported.Labels,
MachineID: reported.MachineID,
IsDpfEnabled: reported.IsDpfEnabled,
Expand All @@ -168,6 +169,7 @@ func (mei ManageExpectedMachine) UpdateExpectedMachinesInDB(ctx context.Context,
!util.PtrsEqual(cur.SkuID, reported.SkuID) ||
!util.PtrsEqual(cur.MachineID, reported.MachineID) ||
!reflect.DeepEqual(cur.FallbackDpuSerialNumbers, reported.FallbackDpuSerialNumbers) ||
!util.PtrsEqual(cur.BmcIpAddress, reported.BmcIpAddress) ||
!reflect.DeepEqual(cur.Labels, reported.Labels) ||
!util.PtrsEqual(cur.IsDpfEnabled, reported.IsDpfEnabled) {
// nil labels in nico can mean we need to clear out existing labels in DB
Expand All @@ -177,18 +179,36 @@ func (mei ManageExpectedMachine) UpdateExpectedMachinesInDB(ctx context.Context,
if cur.Labels != nil && labels == nil {
labels = map[string]string{}
}
_, uerr := emDAO.Update(ctx, nil, cdbm.ExpectedMachineUpdateInput{
ExpectedMachineID: cur.ID,
BmcMacAddress: &reported.BmcMacAddress,
ChassisSerialNumber: &reported.ChassisSerialNumber,
SkuID: reported.SkuID,
MachineID: reported.MachineID,
FallbackDpuSerialNumbers: reported.FallbackDpuSerialNumbers,
Labels: labels,
IsDpfEnabled: reported.IsDpfEnabled,

uerr := cdb.WithTx(ctx, mei.dbSession, func(tx *cdb.Tx) error {
// Passing nil to Update leaves the existing value unchanged, so explicitly clear a
// BMC IP address that NICo no longer reports.
if cur.BmcIpAddress != nil && reported.BmcIpAddress == nil {
_, cerr := emDAO.Clear(ctx, tx, cdbm.ExpectedMachineClearInput{
ExpectedMachineID: cur.ID,
BmcIpAddress: true,
})
if cerr != nil {
return cerr
}
}

_, uerr := emDAO.Update(ctx, tx, cdbm.ExpectedMachineUpdateInput{
ExpectedMachineID: cur.ID,
BmcMacAddress: &reported.BmcMacAddress,
ChassisSerialNumber: &reported.ChassisSerialNumber,
SkuID: reported.SkuID,
MachineID: reported.MachineID,
FallbackDpuSerialNumbers: reported.FallbackDpuSerialNumbers,
BmcIpAddress: reported.BmcIpAddress,
Labels: labels,
IsDpfEnabled: reported.IsDpfEnabled,
})
return uerr
})
if uerr != nil {
logger.Error().Err(uerr).Str("ExpectedMachineID", cur.ID.String()).Msg("failed to update ExpectedMachine in DB")
logger.Error().Err(uerr).Str("ExpectedMachineID", cur.ID.String()).Msg("failed to reconcile ExpectedMachine in DB")
continue
}
}
}
Expand Down
134 changes: 134 additions & 0 deletions rest-api/workflow/pkg/activity/expectedmachine/expectedmachine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/uptrace/bun/extra/bundebug"

cdb "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db"
Expand Down Expand Up @@ -465,6 +466,139 @@ func TestManageExpectedMachine_UpdateExpectedMachinesInDB(t *testing.T) {
}
}

func TestManageExpectedMachine_UpdateExpectedMachinesInDB_BmcIpAddress(t *testing.T) {
ctx := context.Background()

dbSession := testExpectedMachineInitDB(t)
defer dbSession.Close()

testExpectedMachineSetupSchema(t, dbSession)

ipOrg := "test-provider-org"
ipRoles := []string{"FORGE_PROVIDER_ADMIN"}
ipu := cwu.TestBuildUser(t, dbSession, uuid.NewString(), []string{ipOrg}, ipRoles)
ip := cwu.TestBuildInfrastructureProvider(t, dbSession, "test-provider", ipOrg, ipu)
st := cwu.TestBuildSite(t, dbSession, ip, "test-site-bmc-ip", cdbm.SiteStatusRegistered, nil, ipu)

type existingMachineCase struct {
name string
initialBmcIP *string
reportedBmcIP *string
}

tests := []existingMachineCase{
{
name: "set previously unset BMC IP address",
initialBmcIP: nil,
reportedBmcIP: cutil.GetPtr("192.0.2.10"),
},
{
name: "change existing BMC IP address",
initialBmcIP: cutil.GetPtr("192.0.2.20"),
reportedBmcIP: cutil.GetPtr("192.0.2.21"),
},
{
name: "clear existing BMC IP address",
initialBmcIP: cutil.GetPtr("192.0.2.30"),
reportedBmcIP: nil,
},
}

emDAO := cdbm.NewExpectedMachineDAO(dbSession)
existingIDs := make([]uuid.UUID, len(tests))
reportedMachines := make([]*corev1.ExpectedMachine, 0, len(tests)+1)
preservedName := "preserve-this-name"
for i, tt := range tests {
emID := uuid.New()
existingIDs[i] = emID
bmcMAC := fmt.Sprintf("00:11:22:33:66:%02d", i)
chassisSerialNumber := fmt.Sprintf("BMC-IP-SN-%d", i)
_, err := emDAO.Create(ctx, nil, cdbm.ExpectedMachineCreateInput{
ExpectedMachineID: emID,
SiteID: st.ID,
BmcMacAddress: bmcMAC,
ChassisSerialNumber: chassisSerialNumber,
BmcIpAddress: tt.initialBmcIP,
Name: &preservedName,
CreatedBy: ipu.ID,
})
require.NoError(t, err)

reportedMachines = append(reportedMachines, &corev1.ExpectedMachine{
Id: &corev1.UUID{Value: emID.String()},
BmcMacAddress: bmcMAC,
ChassisSerialNumber: chassisSerialNumber,
BmcIpAddress: tt.reportedBmcIP,
})
}

createdID := uuid.New()
createdBmcIP := "192.0.2.40"
reportedMachines = append(reportedMachines, &corev1.ExpectedMachine{
Id: &corev1.UUID{Value: createdID.String()},
BmcMacAddress: "00:11:22:33:66:03",
ChassisSerialNumber: "BMC-IP-SN-3",
BmcIpAddress: &createdBmcIP,
})

mei := ManageExpectedMachine{dbSession: dbSession}
err := mei.UpdateExpectedMachinesInDB(ctx, st.ID, &corev1.ExpectedMachineInventory{
ExpectedMachines: reportedMachines,
Timestamp: timestamppb.Now(),
InventoryStatus: corev1.InventoryStatus_INVENTORY_STATUS_SUCCESS,
})
require.NoError(t, err)

for i, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
updated, err := emDAO.Get(ctx, nil, existingIDs[i], nil, false)
require.NoError(t, err)
assert.Equal(t, tt.reportedBmcIP, updated.BmcIpAddress)
assert.Equal(t, &preservedName, updated.Name, "reconciling the BMC IP must preserve unrelated fields")
})
}

created, err := emDAO.Get(ctx, nil, createdID, nil, false)
require.NoError(t, err)
assert.Equal(t, &createdBmcIP, created.BmcIpAddress)

t.Run("rolls back a clear when the accompanying update fails", func(t *testing.T) {
emID := uuid.New()
originalBmcIP := "192.0.2.50"
_, err := emDAO.Create(ctx, nil, cdbm.ExpectedMachineCreateInput{
ExpectedMachineID: emID,
SiteID: st.ID,
BmcMacAddress: "00:11:22:33:66:04",
ChassisSerialNumber: "BMC-IP-SN-4",
BmcIpAddress: &originalBmcIP,
Name: &preservedName,
CreatedBy: ipu.ID,
})
require.NoError(t, err)

nonexistentSkuID := "nonexistent-sku"
err = mei.UpdateExpectedMachinesInDB(ctx, st.ID, &corev1.ExpectedMachineInventory{
ExpectedMachines: []*corev1.ExpectedMachine{
{
Id: &corev1.UUID{Value: emID.String()},
BmcMacAddress: "00:11:22:33:66:04",
ChassisSerialNumber: "BMC-IP-SN-4",
BmcIpAddress: nil,
SkuId: &nonexistentSkuID,
},
},
Timestamp: timestamppb.Now(),
InventoryStatus: corev1.InventoryStatus_INVENTORY_STATUS_SUCCESS,
})
require.NoError(t, err, "per-record reconciliation errors remain best effort")

unchanged, err := emDAO.Get(ctx, nil, emID, nil, false)
require.NoError(t, err)
assert.Equal(t, &originalBmcIP, unchanged.BmcIpAddress)
assert.Equal(t, &preservedName, unchanged.Name)
})
}

func TestManageExpectedMachine_UpdateExpectedMachinesInDB_RaceCondition(t *testing.T) {
ctx := context.Background()

Expand Down
Loading