Skip to content

OADP-7895 Fix for the oc oadp nonadmin backup describe shows Pod Volu… - #262

Merged
openshift-merge-bot[bot] merged 2 commits into
migtools:oadp-devfrom
SharoonAustin06:OADP-7895
Sep 22, 2026
Merged

openshift-merge-bot[bot] merged 2 commits into
migtools:oadp-devfrom
SharoonAustin06:OADP-7895

Conversation

@SharoonAustin06

@SharoonAustin06 SharoonAustin06 commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Why the changes were made

For the non-admin command whenever we are describing the backup it simply returns a static string, with these changes we have made it dynamic.

How to test the changes made

Create a non-admin backup for the non-admin user and try to use the oadp nonadmin describe commands with and without details flag.

Summary by CodeRabbit

  • New Features

    • Backup descriptions now include a compact breakdown of backed-up pod volumes, including pod, namespace, volume name, and size.
    • When detailed information is not requested, the output shows a completion count and guidance to use --details for the full breakdown.
  • Bug Fixes

    • Detailed backup information is fetched once and reused, avoiding duplicate requests and improving command efficiency.

@openshift-ci
openshift-ci Bot requested review from sseago and weshayutin September 3, 2026 06:58
@coderabbitai

coderabbitai Bot commented Sep 3, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: fe5a591c-a887-4b4f-a8ed-3599ea8d1c44

📥 Commits

Reviewing files that changed from the base of the PR and between 5080bb6 and 950c98f.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (4)
  • Containerfile.download
  • cmd/non-admin/backup/describe.go
  • cmd/non-admin/backup/describe_test.go
  • go.mod

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change upgrades the Go toolchain and dependencies. The non-admin backup description command now reuses fetched volume information and displays compact pod-volume backup details.

Changes

Backup description and toolchain updates

Layer / File(s) Summary
Go toolchain and dependency updates
Containerfile.download, go.mod
The builder and module configuration now use Go 1.26. Velero, Kubernetes, controller-runtime, YAML, and related dependencies were updated.
Backup detail retrieval and output
cmd/non-admin/backup/describe.go
The command pre-fetches BackupVolumeInfos when --details is set and passes the result to both detail renderers. Pod-volume output now shows counts, hints, and compact details.
Pod-volume formatting validation
cmd/non-admin/backup/describe.go, cmd/non-admin/backup/describe_test.go
New public volume-info structs and a formatter parse, filter, group, sort, and render successful pod-volume entries. Tests cover valid, empty, malformed, and unsorted input.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant RunE
  participant ProcessDownloadRequest
  participant BackupDetailRenderer
  participant CompactFormatter
  RunE->>ProcessDownloadRequest: request BackupVolumeInfos
  ProcessDownloadRequest-->>RunE: return volumeInfo
  RunE->>BackupDetailRenderer: render backup details with volumeInfo
  BackupDetailRenderer->>CompactFormatter: format pod-volume entries
  CompactFormatter-->>BackupDetailRenderer: return compact output
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title identifies the non-admin backup describe fix and the pod volume output issue. The ending is truncated, but the title remains related and sufficiently specific.
Description check ✅ Passed The description includes both required sections. It explains the problem and gives a basic test procedure using non-admin backup describe commands with and without the details flag.
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (2 skipped: 2 u…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Warning

⚠️ This pull request shows signs of AI-generated slop (redundant_comments, description_diff_mismatch). It has been flagged by CodeRabbit slop detection and should be reviewed carefully.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@openshift-ci

openshift-ci Bot commented Sep 3, 2026

Copy link
Copy Markdown

Hi @SharoonAustin06. Thanks for your PR.

I'm waiting for a migtools member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Comment thread cmd/non-admin/backup/describe.go Outdated
Comment thread cmd/non-admin/backup/describe.go Outdated
Comment thread cmd/non-admin/backup/describe.go
Comment thread go.mod Outdated
@SharoonAustin06
SharoonAustin06 force-pushed the OADP-7895 branch 2 times, most recently from 42983d1 to d5ac56d Compare September 7, 2026 11:54
Comment thread cmd/non-admin/backup/describe_test.go Outdated
Comment thread cmd/non-admin/backup/describe_test.go Outdated
Comment thread cmd/non-admin/backup/describe_test.go Outdated
@SharoonAustin06
SharoonAustin06 force-pushed the OADP-7895 branch 2 times, most recently from 954dd0f to e685235 Compare September 11, 2026 05:21

@shubham-pampattiwar shubham-pampattiwar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm

@shubham-pampattiwar

Copy link
Copy Markdown
Member

/cherry-pick oadp-1.6

@openshift-cherrypick-robot

Copy link
Copy Markdown

@shubham-pampattiwar: once the present PR merges, I will cherry-pick it on top of oadp-1.6 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick oadp-1.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@shubham-pampattiwar

Copy link
Copy Markdown
Member

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Sep 16, 2026
sseago
sseago previously approved these changes Sep 16, 2026
@sseago

sseago commented Sep 16, 2026

Copy link
Copy Markdown

/retest

@SharoonAustin06

Copy link
Copy Markdown
Contributor Author

/retest-required

@openshift-ci

openshift-ci Bot commented Sep 17, 2026

Copy link
Copy Markdown

@SharoonAustin06: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/retest-required

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@sseago

sseago commented Sep 17, 2026

Copy link
Copy Markdown

/ok-to-test

@sseago

sseago commented Sep 17, 2026

Copy link
Copy Markdown

/retest

@sseago

sseago commented Sep 17, 2026

Copy link
Copy Markdown

/retest-required

@sseago

sseago commented Sep 18, 2026

Copy link
Copy Markdown

/retest

@sseago

sseago commented Sep 18, 2026

Copy link
Copy Markdown

/retest

@SharoonAustin06

Copy link
Copy Markdown
Contributor Author

/retest-required

@kaovilai

Copy link
Copy Markdown
Member

/test images

go.mod now requires go >= 1.26.0, but the image build stage still
pulled golang:1.25, failing go mod download with:
"go.mod requires go >= 1.26.0 (running go 1.25.14; GOTOOLCHAIN=local)"

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Tiger Kaovilai <passawit.kaovilai@gmail.com>
@kaovilai
kaovilai dismissed stale reviews from sseago and shubham-pampattiwar via 950c98f September 21, 2026 18:57
@openshift-ci openshift-ci Bot removed the lgtm label Sep 21, 2026
@sseago

sseago commented Sep 21, 2026

Copy link
Copy Markdown

/lgtm

@openshift-ci

openshift-ci Bot commented Sep 22, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SharoonAustin06, shubham-pampattiwar, sseago

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [shubham-pampattiwar,sseago]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot
openshift-merge-bot Bot merged commit 561ec1d into migtools:oadp-dev Sep 22, 2026
13 checks passed
@openshift-cherrypick-robot

Copy link
Copy Markdown

@shubham-pampattiwar: #262 failed to apply on top of branch "oadp-1.6":

Applying: OADP-7895 Fix for the oc oadp nonadmin backup describe shows Pod Volume Backups: <none included>
Using index info to reconstruct a base tree...
M	go.mod
M	go.sum
Falling back to patching base and 3-way merge...
Auto-merging go.mod
CONFLICT (content): Merge conflict in go.mod
Auto-merging go.sum
CONFLICT (content): Merge conflict in go.sum
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 OADP-7895 Fix for the oc oadp nonadmin backup describe shows Pod Volume Backups: <none included>

Details

In response to this:

/cherry-pick oadp-1.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@shubham-pampattiwar

Copy link
Copy Markdown
Member

@SharoonAustin06 Auto CP failed, please create a manual CP PR.

@SharoonAustin06

Copy link
Copy Markdown
Contributor Author

/cherry-pick oadp-1.6

@openshift-cherrypick-robot

Copy link
Copy Markdown

@SharoonAustin06: #262 failed to apply on top of branch "oadp-1.6":

Applying: OADP-7895 Fix for the oc oadp nonadmin backup describe shows Pod Volume Backups: <none included>
Using index info to reconstruct a base tree...
M	go.mod
M	go.sum
Falling back to patching base and 3-way merge...
Auto-merging go.mod
CONFLICT (content): Merge conflict in go.mod
Auto-merging go.sum
CONFLICT (content): Merge conflict in go.sum
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 OADP-7895 Fix for the oc oadp nonadmin backup describe shows Pod Volume Backups: <none included>

Details

In response to this:

/cherry-pick oadp-1.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants