feat: Add Rollout actions (#8568) - #257
Conversation
Signed-off-by: Keith Chong <kykchong@redhat.com>
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesRollout revisions workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Revisions
participant ActionProvider
participant RolloutService
participant Kubernetes
participant AlertGroup
User->>Revisions: Select rollout or revision action
Revisions->>ActionProvider: Build enabled action
ActionProvider->>RolloutService: Invoke rollout patch helper
RolloutService->>Kubernetes: Submit JSON-Patch
Kubernetes-->>RolloutService: Return patched rollout
RolloutService-->>Revisions: Resolve or report error
Revisions->>AlertGroup: Display action error
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #257 +/- ##
==========================================
- Coverage 11.92% 11.63% -0.29%
==========================================
Files 154 160 +6
Lines 6272 6488 +216
Branches 2028 2221 +193
==========================================
+ Hits 748 755 +7
+ Misses 5524 5502 -22
- Partials 0 231 +231
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
src/gitops/components/rollout/RolloutStatus.tsx (1)
15-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
statusis now used as optional; update the type.The component explicitly handles
status === undefined(line 51) and callers passobj.status?.phase as RolloutStatus, casting awayundefined. Makingstatusoptional avoids relying onascasts to bypass the type checker.🩹 Proposed fix
interface RolloutStatusProps { - status: RolloutStatus; + status?: RolloutStatus; message?: string; showPhaseLabel?: boolean; }🤖 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 `@src/gitops/components/rollout/RolloutStatus.tsx` around lines 15 - 25, Update RolloutStatusProps so status is optional, matching RolloutStatusFragment’s existing handling of an undefined status and allowing callers to pass optional phase values without casts. Preserve the current rendering behavior for defined statuses and the status === undefined branch.src/gitops/components/rollout/revisions/Revisions.tsx (1)
394-470: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftToolbar buttons duplicate the dropdown action logic and skip RBAC gating.
Promote/FullPromote/Abort/Retry/Restart here re-implement the same
isDisabledconditions and service calls already defined inuseRolloutRevisionsActionsProvider(used forRolloutActionsCellright above). Two risks: (1) the two implementations can drift over time, and (2) unlike the dropdown'sActionDropdownItem(which enforcesaccessReviewviauseAccessReview), these buttons have no permission-based disabling, so users without patch rights see enabled buttons that always fail server-side.Consider driving these buttons off the same
useRolloutRevisionsActionsProvider(rollout, onRevisionError)actions array (mappingid→ button) to keep behavior and RBAC gating in one place.🤖 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 `@src/gitops/components/rollout/revisions/Revisions.tsx` around lines 394 - 470, Update the toolbar in Revisions to use the action definitions returned by useRolloutRevisionsActionsProvider(rollout, onRevisionError) instead of duplicating promoteRollout, abortRollout, retryRollout, and restartRollout handlers and disabled conditions. Map each action by its id to the corresponding button, preserving labels and button layout while deriving disabled state and invocation from the shared actions so accessReview/RBAC gating remains consistent with RolloutActionsCell.
🤖 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 `@src/gitops/components/rollout/hooks/useRolloutRevisionsActionsProvider.tsx`:
- Around line 64-68: Update the separator item in
useRolloutRevisionsActionsProvider so its cta handler returns undefined without
using an empty function, preserving the separator’s existing behavior while
satisfying the no-empty-function lint rule.
In
`@src/gitops/components/rollout/hooks/useRolloutRevisionsRSActionsProvider.tsx`:
- Around line 26-45: Update the Rollback action in the
useRolloutRevisionsRSActionsProvider hook to disable it when
replicaSet.replicaSet is missing, in addition to the existing index === 0
condition. Preserve rollbackRollout invocation for valid ReplicaSet entries and
keep the current error handling unchanged.
In `@src/gitops/components/rollout/revisions/ReplicaSetInfo.ts`:
- Line 188: Update the replicaSetScaleDownDeadline access in getReplicaSetInfo
to safely handle missing metadata.annotations by applying optional chaining to
annotations before reading the scale-down-deadline key, while preserving the
existing value when annotations are present.
In `@src/gitops/components/rollout/revisions/Revisions.scss`:
- Around line 1-6: Add the required blank line before the first declaration in
the .gitops-revisions-table tbody > tr:first-child rule to satisfy Stylelint’s
declaration-empty-line-before rule.
In `@src/gitops/components/rollout/revisions/Revisions.tsx`:
- Around line 159-166: Update onRevisionError to keep the user-facing
RevisionAlert.message concise and move error.stack into RevisionAlert.details
instead of exposing raw traces in the toast. Ensure the alert presentation
renders details only through its existing expandable/details mechanism,
preserving stack visibility for users who explicitly expand it.
In `@src/gitops/components/rollout/RolloutNavPage.tsx`:
- Around line 39-40: Update the revision actions provider call in RolloutNavPage
to pass the existing revision alert mechanism as its onError callback. Ensure
failures consumed by useRolloutRevisionsActionsProvider are surfaced to users
while preserving the current action setup.
- Around line 38-41: Update RolloutNavPage’s action selection to use the
pathname returned by useLocation(), then invoke both
useRolloutRevisionsActionsProvider and useRolloutActionsProvider unconditionally
and select the appropriate returned array based on the revisions-route check.
Preserve the existing route-sensitive action behavior while keeping hook calls
in a stable order.
In `@src/gitops/components/rollout/services/Rollout.ts`:
- Around line 42-45: Update the JSON Patch operations for the optional restartAt
and pauseConditions fields in Rollout to use `add` instead of `replace`, so
patches succeed whether the target members are absent or already present.
In `@src/gitops/utils/utils.tsx`:
- Around line 213-254: Update formatDuration to use inclusive day, hour, and
minute thresholds so exact multiples render in their largest applicable unit
(for example, 3600 seconds as hours). Adjust the sigfigs truncation path to join
selected segments without appending a trailing space, while preserving existing
rounding and formatting behavior.
---
Nitpick comments:
In `@src/gitops/components/rollout/revisions/Revisions.tsx`:
- Around line 394-470: Update the toolbar in Revisions to use the action
definitions returned by useRolloutRevisionsActionsProvider(rollout,
onRevisionError) instead of duplicating promoteRollout, abortRollout,
retryRollout, and restartRollout handlers and disabled conditions. Map each
action by its id to the corresponding button, preserving labels and button
layout while deriving disabled state and invocation from the shared actions so
accessReview/RBAC gating remains consistent with RolloutActionsCell.
In `@src/gitops/components/rollout/RolloutStatus.tsx`:
- Around line 15-25: Update RolloutStatusProps so status is optional, matching
RolloutStatusFragment’s existing handling of an undefined status and allowing
callers to pass optional phase values without casts. Preserve the current
rendering behavior for defined statuses and the status === undefined branch.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c07b52cf-0502-4a89-88f5-1c0c70ae5fad
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
locales/en/plugin__gitops-plugin.jsonlocales/ja/plugin__gitops-plugin.jsonlocales/ko/plugin__gitops-plugin.jsonlocales/zh/plugin__gitops-plugin.jsonpackage.jsonsrc/gitops/components/rollout/RolloutDetailsTab.tsxsrc/gitops/components/rollout/RolloutList.tsxsrc/gitops/components/rollout/RolloutNavPage.tsxsrc/gitops/components/rollout/RolloutRevisionsTab.tsxsrc/gitops/components/rollout/RolloutStatus.tsxsrc/gitops/components/rollout/hooks/useRolloutActionsProvider.tsxsrc/gitops/components/rollout/hooks/useRolloutRevisionsActionsProvider.tsxsrc/gitops/components/rollout/hooks/useRolloutRevisionsRSActionsProvider.tsxsrc/gitops/components/rollout/revisions/ReplicaSetInfo.tssrc/gitops/components/rollout/revisions/Revisions.scsssrc/gitops/components/rollout/revisions/Revisions.tsxsrc/gitops/components/rollout/services/Rollout.tssrc/gitops/components/shared/Ticker/Ticker.tsxsrc/gitops/utils/components/ActionDropDown/ActionDropDown.tsxsrc/gitops/utils/components/Icons/Icons.tsxsrc/gitops/utils/utils.tsx
💤 Files with no reviewable changes (1)
- src/gitops/components/rollout/hooks/useRolloutActionsProvider.tsx
|
Yes I made a mess in my computer. And I just lost a lot of stuff
…On Thu, Jul 30, 2026 at 5:10 PM coderabbitai[bot] ***@***.***> wrote:
***@***.***[bot]* commented on this pull request.
------------------------------
In src/gitops/components/rollout/services/Rollout.ts
<#257 (comment)>
:
> + op: 'replace',
+ path: '/spec/restartAt',
+ value: now,
+ },
@keithchong Understood—I'll leave this as-is. The concern only applies if
either target field can be absent; if the intended contract is to require
the existing fields before these actions run, replace is appropriate.
You are interacting with an AI system.
—
Reply to this email directly, view it on GitHub
<#257?email_source=notifications&email_token=CH7J3Z2Q2WOLZK7AMM2ZF7L5HO2VHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIOBSGMZDKNBRGM22M4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOSWGM33PORSXEX3DNRUWG2Y#discussion_r3686266439>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/CH7J3Z336Y5T2IBI4CXJQJD5HO2VHAVCNFSNUABFKJSXA33TNF2G64TZHM2TCMZWGQYDQMZUHNEXG43VMU5TKMBSGI4DGOJSGE32C5QC>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/CH7J3ZYJR7QJNGGZ4MRSC6L5HO2VHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIOBSGMZDKNBRGM22M4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOSVGM33PORSXEX3JN5ZQ>
and Android
<https://github.com/notifications/mobile/android/CH7J3Z3SKBUK7L2J2ENFPRL5HO2VHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIOBSGMZDKNBRGM22M4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOSXGM33PORSXEX3BNZSHE33JMQ>.
Download it today!
You are receiving this because you are subscribed to this thread.Message
ID: <redhat-developer/gitops-console-plugin/pull/257/review/4823254135@
github.com>
|
Signed-off-by: Keith Chong <kykchong@redhat.com>
Signed-off-by: Keith Chong <kykchong@redhat.com>
aali309
left a comment
aali309
left a comment
There was a problem hiding this comment.
Thanks @keithchong , left a few comments
| readyCount++; | ||
| } | ||
| }); | ||
| podsChildren.push({ |
There was a problem hiding this comment.
Align (current 5 cells) with parent (6 rows actions column). May be we add an empty 6th cell so the tree table stays aligned?
| "git-url-parse": "^13.1.0", | ||
| "lodash": "^4.18.1", | ||
| "moment": "^2.29.4", | ||
| "pnpm": "^11.18.0", |
There was a problem hiding this comment.
"pnpm": "^11.18.0" conflicts with packageManager: pnpm@10.33.2 and breaks pnpm run start under Corepack. Do we want to remove from deps?
| promoteRollout(rollout, true).catch((err: unknown) => { | ||
| onRevisionError( | ||
| err instanceof Error ? err : String(err), | ||
| t('FullPromote'), |
There was a problem hiding this comment.
nit: Use t('Full Promote') instead of t('FullPromote') to match the button label / locale key.
4 participants
See [GITOPS-8568] (https://redhat.atlassian.net/browse/GITOPS-8568)