Skip to content

fix(ctl): entry info fields silently falling back to default values#319

Open
iamjoemccormick wants to merge 1 commit intomainfrom
iamjoe/fix/silently-unpopulated-entry-info-fields
Open

fix(ctl): entry info fields silently falling back to default values#319
iamjoemccormick wants to merge 1 commit intomainfrom
iamjoe/fix/silently-unpopulated-entry-info-fields

Conversation

@iamjoemccormick
Copy link
Copy Markdown
Member

What does this PR do / why do we need it?

Required for all PRs.

The new errors surfaced when using the GetEntryInfoV2 ioctl and subsequent changes in #317 prompted the realization that beegfs entry infoonly worked before because newEntry() was never checking if entryInfo msg.GetEntryInfoResponseactually contained valid data (based on entryInfo.Result). At best this returned misleading output in CTL, at worst there are other places that make assumptions based on the validity of that data.

Before when GetEntryInfoResponse failed (e.g., OpsErr_INODELOCKED during chunk rebalancing), fields
derived from that response previously defaulted to Go zero values, causing consumers like 'beegfs
entry info' to silently display misleading data (e.g., Chunksize: 0, Reading: 0, Writing: 0).

This change extracts Pattern, Remote, NumSessionsRead, NumSessionsWrite, and FileState into a new
EntryDetails struct. Entry.Details is a pointer that is nil when EntryInfoPopulated != SUCCESS, so:

  • Display functions gate all response-derived output behind a single nil check, showing
    (unavailable) instead of zero values.
  • Backend consumers (set, migrate, create, RST) get explicit nil guards that surface errors instead
    of silently operating on empty data.
  • Future fields added to EntryDetails are automatically handled by the display layer without
    requiring frontend changes.

Also introduces ErrEntryDetailsUnavailable in the migrate/chooser path, replacing the previous
behavior where a locked inode silently produced zero-value fields that triggered
ErrEntryHasNoTargets.

Assisted-by: Claude:claude-opus-4-6

Related Issue(s)

Required when applicable.

Where should the reviewer(s) start reviewing this?

Only required for larger PRs when this may not be immediately obvious.

Are there any specific topics we should discuss before merging?

Not required.

What are the next steps after this PR?

Not required.

Checklist before merging:

Required for all PRs.

When creating a PR these are items to keep in mind that cannot be checked by GitHub actions:

  • Documentation:
    • Does developer documentation (code comments, readme, etc.) need to be added or updated?
    • Does the user documentation need to be expanded or updated for this change?
  • Testing:
    • Does this functionality require changing or adding new unit tests?
    • Does this functionality require changing or adding new integration tests?
  • Git Hygiene:

For more details refer to the Go coding standards and the pull request process.

The new errors surfaced when using the `GetEntryInfoV2` ioctl and subsequent changes in
#317 prompted the realization that `beegfs entry info`
only worked before because `newEntry()` was never checking if `entryInfo msg.GetEntryInfoResponse`
actually contained valid data (based on `entryInfo.Result`). At best this returned misleading output
in CTL, at worst there are other places that make assumptions based on the validity of that data.

Before when GetEntryInfoResponse failed (e.g., OpsErr_INODELOCKED during chunk rebalancing), fields
derived from that response previously defaulted to Go zero values, causing consumers like 'beegfs
entry info' to silently display misleading data (e.g., Chunksize: 0, Reading: 0, Writing: 0).

This change extracts Pattern, Remote, NumSessionsRead, NumSessionsWrite, and FileState into a new
EntryDetails struct. Entry.Details is a pointer that is nil when EntryInfoPopulated != SUCCESS, so:

- Display functions gate all response-derived output behind a single nil check, showing
  (unavailable) instead of zero values.
- Backend consumers (set, migrate, create, RST) get explicit nil guards that surface errors instead
  of silently operating on empty data.
- Future fields added to EntryDetails are automatically handled by the display layer without
  requiring frontend changes.

Also introduces ErrEntryDetailsUnavailable in the migrate/chooser path, replacing the previous
behavior where a locked inode silently produced zero-value fields that triggered
ErrEntryHasNoTargets.

Assisted-by: Claude:claude-opus-4-6
@iamjoemccormick iamjoemccormick self-assigned this Apr 17, 2026
@iamjoemccormick iamjoemccormick requested a review from a team as a code owner April 17, 2026 23:23
@iamjoemccormick iamjoemccormick added bug Something isn't working ctl Issues primarily affecting the BeeGFS control tool. labels Apr 17, 2026
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ctl Issues primarily affecting the BeeGFS control tool.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant