fix: Delete InfiniBand Interfaces based on config sync flag instead of Instance status#567
Open
hwadekar-nv wants to merge 2 commits into
Open
fix: Delete InfiniBand Interfaces based on config sync flag instead of Instance status#567hwadekar-nv wants to merge 2 commits into
hwadekar-nv wants to merge 2 commits into
Conversation
Contributor
Summary by CodeRabbit
WalkthroughThis PR implements status-aware reconciliation for InfiniBand interfaces in the API handler and corresponding deletion-timing refinements in the workflow activity layer. The handler now defers interface reissue when recent Ready/Pending/Deleting interfaces exist within a 30-second grace window and match the requested slots. Existing Ready interfaces are preserved rather than unconditionally deleted and recreated. Workflow activity adjusts when interfaces in Deleting state become eligible for removal, respecting sync state and stale-inventory thresholds. ChangesInfiniBand Interface Reconciliation with Status-Aware Grace Windows
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-05-22 01:28:36 UTC | Commit: 8dc6ce0 |
Test Results9 579 tests +3 9 579 ✅ +3 7m 32s ⏱️ +39s Results for commit 7b8cc98. ± Comparison against base commit 7cb0298. This pull request removes 1 and adds 4 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workflow/pkg/activity/instance/instance.go (1)
520-536:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard InfiniBand status indexing to prevent panic on config/status mismatch.
Line 535 indexes
controllerInstance.Status.Infiniband.IbInterfaces[idx]without checking length parity with config entries. A shorter status slice will panic and abort inventory processing.💡 Suggested fix
for idx, interfaceConfig := range controllerInstance.Config.Infiniband.IbInterfaces { // Skip if the InfiniBand Interface Config is nil if interfaceConfig == nil { logger.Warn().Int("Index", idx).Msg("InfiniBand Interface Config is nil, skipping update") continue } @@ - interfaceStatus := controllerInstance.Status.Infiniband.IbInterfaces[idx] + if controllerInstance.Status.Infiniband == nil || idx >= len(controllerInstance.Status.Infiniband.IbInterfaces) { + slogger.Warn().Int("Index", idx).Msg("InfiniBand Interface Status missing for config entry, skipping update") + continue + } + interfaceStatus := controllerInstance.Status.Infiniband.IbInterfaces[idx] if interfaceStatus != nil {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workflow/pkg/activity/instance/instance.go` around lines 520 - 536, The code indexes controllerInstance.Status.Infiniband.IbInterfaces[idx] without ensuring the status slice is at least as long as controllerInstance.Config.Infiniband.IbInterfaces; update the loop around controllerInstance.Config.Infiniband.IbInterfaces (using idx) to first check that idx < len(controllerInstance.Status.Infiniband.IbInterfaces) and that the status entry is non-nil before assigning interfaceStatus, and if the check fails log a warning (using logger) and continue or create a default status entry so the subsequent logic that uses interfaceStatus and infiniBandInterfaceMap lookup cannot panic.
🧹 Nitpick comments (3)
api/pkg/api/handler/instance.go (1)
3036-3040: 💤 Low valueReconciliation condition includes Deleting interfaces, potentially inconsistent with NVLink pattern.
The
syncedIbIfcSlotscalculation includesexistingDeletingIbIfcsCount, allowing interfaces in Deleting state to satisfy the request count. This differs from the NVLink logic at line 3230, which explicitly triggers reissue whenexistingDeletingNvlIfcsCount != 0.If this behavioral difference is intentional (e.g., InfiniBand deletions are expected to be re-requested by workflow activity), consider adding a comment explaining the rationale. Otherwise, consider aligning with the NVLink pattern:
syncedIbIfcSlots := existingReadyIbIfcsCount + existingPendingIbIfcsCount🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/pkg/api/handler/instance.go` around lines 3036 - 3040, The reconciliation currently counts interfaces in Deleting state toward syncedIbIfcSlots (using existingDeletingIbIfcsCount) which diverges from the NVLink pattern that treats deleting NVLink interfaces as a trigger to reissue (existingDeletingNvlIfcsCount); update the InfiniBand logic in the block that computes syncedIbIfcSlots and sets reIssueInfiniBandInterfaces so it either excludes existingDeletingIbIfcsCount (i.e., use existingReadyIbIfcsCount + existingPendingIbIfcsCount and compare to len(apiRequest.InfiniBandInterfaces)) to match NVLink behavior, or if keeping deletions is intentional add a clear comment next to reIssueInfiniBandInterfaces and the syncedIbIfcSlots calculation explaining why Deleting interfaces should satisfy the request; reference reIssueInfiniBandInterfaces, syncedIbIfcSlots, existingReadyIbIfcsCount, existingPendingIbIfcsCount, existingDeletingIbIfcsCount and existingDeletingNvlIfcsCount when making the change.api/pkg/api/handler/instance_test.go (2)
6445-6472: ⚡ Quick winStrengthen no-op assertions by checking zero Deleting IB rows.
For no-op scenarios, asserting only “no Pending” can still miss accidental transitions to
Deleting. Add an explicitDeletingcount assertion in the same block.Proposed test-hardening patch
if len(tt.args.expectInfiniBandInterfacesRemainReady) > 0 { ibiDAO := cdbm.NewInfiniBandInterfaceDAO(tt.fields.dbSession) pendingIb, _, ierr := ibiDAO.GetAll(ec.Request().Context(), nil, cdbm.InfiniBandInterfaceFilterInput{InstanceIDs: []uuid.UUID{reqIns.ID}, Statuses: []string{cdbm.InfiniBandInterfaceStatusPending}}, cdbp.PageInput{}, nil) require.NoError(t, ierr) assert.Len(t, pendingIb, 0, "READY InfiniBand no-op must not insert Pending rows when partition+device+deviceInstance matches existing READY interfaces") + + deletingIb, _, derr := ibiDAO.GetAll(ec.Request().Context(), nil, + cdbm.InfiniBandInterfaceFilterInput{InstanceIDs: []uuid.UUID{reqIns.ID}, + Statuses: []string{cdbm.InfiniBandInterfaceStatusDeleting}}, + cdbp.PageInput{}, nil) + require.NoError(t, derr) + assert.Len(t, deletingIb, 0, "READY InfiniBand no-op must not transition rows to Deleting") for _, wantID := range tt.args.expectInfiniBandInterfacesRemainReady { ibRow, ierr := ibiDAO.GetByID(ec.Request().Context(), nil, wantID, nil)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/pkg/api/handler/instance_test.go` around lines 6445 - 6472, The test currently only asserts there are zero Pending InfiniBandInterface rows in the no-op branch; add an additional check that there are zero Deleting rows to prevent accidental transitions to Deleting. In the block guarded by tt.args.expectInfiniBandInterfacesRemainReady, reuse ibiDAO (cdbm.NewInfiniBandInterfaceDAO) and call GetAll with InfiniBandInterfaceFilterInput.Statuses set to []string{cdbm.InfiniBandInterfaceStatusDeleting}, assert NoError on the call and assert.Len(..., 0) for deleting rows, and also ensure the API response rst.InfiniBandInterfaces contains no entries with Status == cdbm.InfiniBandInterfaceStatusDeleting for the expected IDs.
4256-4383: 🏗️ Heavy liftAdd explicit IB grace-window cases for Pending/Deleting rows.
These new cases validate READY no-op behavior well, but they do not exercise the 30-second grace-window branch for InfiniBand Pending/Deleting records. Please add IB-specific fresh-vs-stale cases to lock down the reconciliation contract and avoid regressions in reissue decisions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/pkg/api/handler/instance_test.go` around lines 4256 - 4383, Add two new table-driven test cases in instance_test.go mirroring the existing READY no-op IB cases but targeting Pending and Deleting InfiniBand rows to exercise the 30-second grace-window logic: create one case where an existing InfiniBandInterface row for the instance has Status Pending (or Deleting) with CreatedAt within the 30s grace window (expect no reissue/no-delete behavior, similar to expectInfiniBandInterfacesRemainReady), and another case where the existing Pending/Deleting row is older than 30s (expect it to be treated stale and to be deleted/reissued, e.g., set ibInterfaceToDelete to that row); populate args.reqData.InfiniBandInterfaces with matching InfiniBandPartitionID/Device/DeviceInstance values, set reqInstance to the appropriate instance ID, and assert the expected respCode and verifyChildSpanner/verifySiteControllerRequest flags as in the surrounding tests so the Pending/Deleting branch is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/pkg/api/handler/instance.go`:
- Around line 3084-3086: The response builder is duplicating InfiniBand and
NVLink entries: when reIssueInfiniBandInterfaces is false the code sets
newIbIfcs = existingIbIfcs and later appends existingIbIfcs again, and the same
happens for newNvlIfcs/newNvlIfcs with reIssueNVLinkInterfaces; fix this by
removing the redundant append branches so that when reIssueInfiniBandInterfaces
is false you do not append existingIbIfcs to newIbIfcs (similarly remove the
append of existingNvlIfcs when reIssueNVLinkInterfaces is false), ensuring only
one assignment/collection is used for newIbIfcs and newNvlIfcs.
---
Outside diff comments:
In `@workflow/pkg/activity/instance/instance.go`:
- Around line 520-536: The code indexes
controllerInstance.Status.Infiniband.IbInterfaces[idx] without ensuring the
status slice is at least as long as
controllerInstance.Config.Infiniband.IbInterfaces; update the loop around
controllerInstance.Config.Infiniband.IbInterfaces (using idx) to first check
that idx < len(controllerInstance.Status.Infiniband.IbInterfaces) and that the
status entry is non-nil before assigning interfaceStatus, and if the check fails
log a warning (using logger) and continue or create a default status entry so
the subsequent logic that uses interfaceStatus and infiniBandInterfaceMap lookup
cannot panic.
---
Nitpick comments:
In `@api/pkg/api/handler/instance_test.go`:
- Around line 6445-6472: The test currently only asserts there are zero Pending
InfiniBandInterface rows in the no-op branch; add an additional check that there
are zero Deleting rows to prevent accidental transitions to Deleting. In the
block guarded by tt.args.expectInfiniBandInterfacesRemainReady, reuse ibiDAO
(cdbm.NewInfiniBandInterfaceDAO) and call GetAll with
InfiniBandInterfaceFilterInput.Statuses set to
[]string{cdbm.InfiniBandInterfaceStatusDeleting}, assert NoError on the call and
assert.Len(..., 0) for deleting rows, and also ensure the API response
rst.InfiniBandInterfaces contains no entries with Status ==
cdbm.InfiniBandInterfaceStatusDeleting for the expected IDs.
- Around line 4256-4383: Add two new table-driven test cases in instance_test.go
mirroring the existing READY no-op IB cases but targeting Pending and Deleting
InfiniBand rows to exercise the 30-second grace-window logic: create one case
where an existing InfiniBandInterface row for the instance has Status Pending
(or Deleting) with CreatedAt within the 30s grace window (expect no
reissue/no-delete behavior, similar to expectInfiniBandInterfacesRemainReady),
and another case where the existing Pending/Deleting row is older than 30s
(expect it to be treated stale and to be deleted/reissued, e.g., set
ibInterfaceToDelete to that row); populate args.reqData.InfiniBandInterfaces
with matching InfiniBandPartitionID/Device/DeviceInstance values, set
reqInstance to the appropriate instance ID, and assert the expected respCode and
verifyChildSpanner/verifySiteControllerRequest flags as in the surrounding tests
so the Pending/Deleting branch is covered.
In `@api/pkg/api/handler/instance.go`:
- Around line 3036-3040: The reconciliation currently counts interfaces in
Deleting state toward syncedIbIfcSlots (using existingDeletingIbIfcsCount) which
diverges from the NVLink pattern that treats deleting NVLink interfaces as a
trigger to reissue (existingDeletingNvlIfcsCount); update the InfiniBand logic
in the block that computes syncedIbIfcSlots and sets reIssueInfiniBandInterfaces
so it either excludes existingDeletingIbIfcsCount (i.e., use
existingReadyIbIfcsCount + existingPendingIbIfcsCount and compare to
len(apiRequest.InfiniBandInterfaces)) to match NVLink behavior, or if keeping
deletions is intentional add a clear comment next to reIssueInfiniBandInterfaces
and the syncedIbIfcSlots calculation explaining why Deleting interfaces should
satisfy the request; reference reIssueInfiniBandInterfaces, syncedIbIfcSlots,
existingReadyIbIfcsCount, existingPendingIbIfcsCount,
existingDeletingIbIfcsCount and existingDeletingNvlIfcsCount when making the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1ad655c9-5e99-4ce6-b20b-c731ed7a085c
📒 Files selected for processing (4)
api/pkg/api/handler/instance.goapi/pkg/api/handler/instance_test.goworkflow/pkg/activity/instance/instance.goworkflow/pkg/activity/instance/instance_test.go
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
thossain-nv
changed the title
hwadekar-nv
force-pushed
the
fix/ib-update-inventory
branch
from
3118f71 to
7b8cc98
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/pkg/api/handler/instance.go`:
- Around line 3011-3053: The current logic only verifies every requested
InfiniBand key has a recent matching existing interface but doesn't check for
extra existing (non-deleting) keys that the request removed, so removals are
treated as no-op; update the comparison between apiRequest.InfiniBandInterfaces
and existingIbIfcMap to be bidirectional: build the set of requested keys from
apiRequest.InfiniBandInterfaces and the set of most-recent non-deleting existing
keys from existingIbIfcMap (ignore Status==Deleting), then if the two sets
differ set reIssueInfiniBandInterfaces = true (instead of relying only on
existingReadyIbIfcsCount/existingPendingIbIfcsCount/existingDeletingIbIfcsCount);
apply the identical bidirectional set check to the NVLink path (the code around
reIssueNVLinkInterfaces) so both directions are validated.
In `@workflow/pkg/activity/instance/instance.go`:
- Around line 554-560: The code compares ibifc.Status to
cdbm.InfiniBandInterfaceStatusReady but assigns cdbm.InterfaceStatusReady when
setting status; change the assignment to use cdbm.InfiniBandInterfaceStatusReady
via cdb.GetStrPtr so the comparison and assignment use the same
InfiniBand-specific constant (locate the block referencing
controllerInstance.Status.Infiniband.ConfigsSynced, ibifc.Status, and the status
= cdb.GetStrPtr(...) line).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 01b41ed4-3f4a-4f83-99ac-830a8ca47bf1
📒 Files selected for processing (4)
api/pkg/api/handler/instance.goapi/pkg/api/handler/instance_test.goworkflow/pkg/activity/instance/instance.goworkflow/pkg/activity/instance/instance_test.go
Comment on lines
+3011
to
3053
| for _, apiIbIfc := range apiRequest.InfiniBandInterfaces { | ||
| key := fmt.Sprintf("%s:%s:%d", apiIbIfc.InfiniBandPartitionID, apiIbIfc.Device, apiIbIfc.DeviceInstance) | ||
| existingIbIfcsForKey := existingIbIfcMap[key] | ||
|
|
||
| // Check the status of the most recent InfiniBand interface for this (partition, device, device instance) key. | ||
| if len(existingIbIfcsForKey) > 0 { | ||
| mostRecentIbIfc := existingIbIfcsForKey[len(existingIbIfcsForKey)-1] | ||
| if mostRecentIbIfc.Status == cdbm.InfiniBandInterfaceStatusReady { | ||
| // This interface is already ready, we don't need to re-issue the InfiniBand interface | ||
| existingReadyIbIfcsCount++ | ||
| } else { | ||
| if mostRecentIbIfc.Updated.After(time.Now().Add(-InfiniBandInterfaceStatusSyncGraceWindow)) { | ||
| if mostRecentIbIfc.Status == cdbm.InfiniBandInterfaceStatusPending { | ||
| existingPendingIbIfcsCount++ | ||
| } else if mostRecentIbIfc.Status == cdbm.InfiniBandInterfaceStatusDeleting { | ||
| existingDeletingIbIfcsCount++ | ||
| } | ||
| } else { | ||
| reIssueInfiniBandInterfaces = true | ||
| } | ||
| } | ||
| } else { | ||
| // No existing InfiniBand interface found for this InfiniBand Partition ID, Device and Device Instance | ||
| reIssueInfiniBandInterfaces = true | ||
| } | ||
| } | ||
|
|
||
| newIbIfcs = append(newIbIfcs, *dbibifc) | ||
| // If we're here and we're not re-issuing InfiniBand interfaces, we need to check if the number of existing InfiniBand interfaces in transition is different from the number of InfiniBand interfaces in the request | ||
| // Assumptions: | ||
| // - There can be no more than 4 InfiniBand Interfaces in Ready state | ||
| // - There can be no more than 4 InfiniBand Interfaces in Pending state | ||
| // - There can more than 4 InfiniBand Interfaces in Deleting state, in multiples of 4 | ||
| // - There cannot be Ready and Pending InfiniBand Interfaces at the same time | ||
| // - There cannot be Ready and Deleting InfiniBand Interfaces at the same time | ||
| if !reIssueInfiniBandInterfaces { | ||
| if existingReadyIbIfcsCount > 0 && existingReadyIbIfcsCount != len(apiRequest.InfiniBandInterfaces) { | ||
| reIssueInfiniBandInterfaces = true | ||
| } else if existingPendingIbIfcsCount > 0 && existingPendingIbIfcsCount != len(apiRequest.InfiniBandInterfaces) { | ||
| reIssueInfiniBandInterfaces = true | ||
| } else if existingDeletingIbIfcsCount > 0 && existingDeletingIbIfcsCount != len(apiRequest.InfiniBandInterfaces) { | ||
| reIssueInfiniBandInterfaces = true | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Treat extra active bindings as drift, not a no-op.
These loops only prove that every requested key has a recent match. They never check the inverse, so a request that removes an InfiniBand or NVLink interface is treated as unchanged if the remaining keys still match. In practice, an existing {A,B} binding updated to {A} leaves B in place, and the explicit empty-slice case behaves the same way.
Please compare the requested key set against the most-recent non-deleting existing key set in both directions before keeping reIssueInfiniBandInterfaces / reIssueNVLinkInterfaces false.
💡 Minimal fix shape
+requestedIbKeys := goset.NewSet[string]()
for _, apiIbIfc := range apiRequest.InfiniBandInterfaces {
key := fmt.Sprintf("%s:%s:%d", apiIbIfc.InfiniBandPartitionID, apiIbIfc.Device, apiIbIfc.DeviceInstance)
+ requestedIbKeys.Add(key)
existingIbIfcsForKey := existingIbIfcMap[key]
...
}
if !reIssueInfiniBandInterfaces {
+ for key, rows := range existingIbIfcMap {
+ mostRecent := rows[len(rows)-1]
+ if mostRecent.Status != cdbm.InfiniBandInterfaceStatusDeleting && !requestedIbKeys.Contains(key) {
+ reIssueInfiniBandInterfaces = true
+ break
+ }
+ }
...
}Apply the same bidirectional check to the NVLink path.
Also applies to: 3202-3254
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api/pkg/api/handler/instance.go` around lines 3011 - 3053, The current logic
only verifies every requested InfiniBand key has a recent matching existing
interface but doesn't check for extra existing (non-deleting) keys that the
request removed, so removals are treated as no-op; update the comparison between
apiRequest.InfiniBandInterfaces and existingIbIfcMap to be bidirectional: build
the set of requested keys from apiRequest.InfiniBandInterfaces and the set of
most-recent non-deleting existing keys from existingIbIfcMap (ignore
Status==Deleting), then if the two sets differ set reIssueInfiniBandInterfaces =
true (instead of relying only on
existingReadyIbIfcsCount/existingPendingIbIfcsCount/existingDeletingIbIfcsCount);
apply the identical bidirectional set check to the NVLink path (the code around
reIssueNVLinkInterfaces) so both directions are validated.
Comment on lines
554
to
+560
| if controllerInstance.Status.Infiniband.ConfigsSynced == cwsv1.SyncState_SYNCED { | ||
| status = cdb.GetStrPtr(cdbm.InterfaceStatusReady) | ||
| // If the InfiniBand Config is synced | ||
| isInfiniBandConfigSynced = true | ||
| if ibifc.Status != cdbm.InfiniBandInterfaceStatusReady { | ||
| // If the InfiniBand Interface is not in Ready state, set the status to Ready | ||
| status = cdb.GetStrPtr(cdbm.InterfaceStatusReady) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if InterfaceStatusReady and InfiniBandInterfaceStatusReady have the same value
echo "Checking InterfaceStatusReady constant:"
rg -n 'InterfaceStatusReady\s*=' --type=go -g '!*_test.go'
echo ""
echo "Checking InfiniBandInterfaceStatusReady constant:"
rg -n 'InfiniBandInterfaceStatusReady\s*=' --type=go -g '!*_test.go'Repository: NVIDIA/infra-controller-rest
Length of output: 471
InfiniBand “Ready” constant consistency
Line 557 compares ibifc.Status to cdbm.InfiniBandInterfaceStatusReady, but line 559 assigns cdbm.InterfaceStatusReady. Both are currently "Ready" (no behavior change today), but keeping the constants aligned prevents future divergence.
Proposed fix for consistency
if controllerInstance.Status.Infiniband.ConfigsSynced == cwsv1.SyncState_SYNCED {
// If the InfiniBand Config is synced
isInfiniBandConfigSynced = true
if ibifc.Status != cdbm.InfiniBandInterfaceStatusReady {
// If the InfiniBand Interface is not in Ready state, set the status to Ready
- status = cdb.GetStrPtr(cdbm.InterfaceStatusReady)
+ status = cdb.GetStrPtr(cdbm.InfiniBandInterfaceStatusReady)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if controllerInstance.Status.Infiniband.ConfigsSynced == cwsv1.SyncState_SYNCED { | |
| status = cdb.GetStrPtr(cdbm.InterfaceStatusReady) | |
| // If the InfiniBand Config is synced | |
| isInfiniBandConfigSynced = true | |
| if ibifc.Status != cdbm.InfiniBandInterfaceStatusReady { | |
| // If the InfiniBand Interface is not in Ready state, set the status to Ready | |
| status = cdb.GetStrPtr(cdbm.InterfaceStatusReady) | |
| } | |
| if controllerInstance.Status.Infiniband.ConfigsSynced == cwsv1.SyncState_SYNCED { | |
| // If the InfiniBand Config is synced | |
| isInfiniBandConfigSynced = true | |
| if ibifc.Status != cdbm.InfiniBandInterfaceStatusReady { | |
| // If the InfiniBand Interface is not in Ready state, set the status to Ready | |
| status = cdb.GetStrPtr(cdbm.InfiniBandInterfaceStatusReady) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workflow/pkg/activity/instance/instance.go` around lines 554 - 560, The code
compares ibifc.Status to cdbm.InfiniBandInterfaceStatusReady but assigns
cdbm.InterfaceStatusReady when setting status; change the assignment to use
cdbm.InfiniBandInterfaceStatusReady via cdb.GetStrPtr so the comparison and
assignment use the same InfiniBand-specific constant (locate the block
referencing controllerInstance.Status.Infiniband.ConfigsSynced, ibifc.Status,
and the status = cdb.GetStrPtr(...) line).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Type of Change
Services Affected
Related Issues (Optional)
Breaking Changes
Testing
Additional Notes