feat: Add Rollout actions (#8568) - #257
Conversation
Signed-off-by: Keith Chong <kykchong@redhat.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 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.61% -0.32%
==========================================
Files 154 161 +7
Lines 6272 6502 +230
Branches 2028 2221 +193
==========================================
+ Hits 748 755 +7
- Misses 5524 5747 +223
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:
|
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
There was a problem hiding this comment.
Thanks @keithchong , left a few comments
Signed-off-by: Keith Chong <kykchong@redhat.com>
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)
src/gitops/components/rollout/hooks/useRolloutRevisionsActionsProvider.tsx (1)
33-36: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPropagate revision-action failures to a user-visible error handler.
RolloutNavPage.tsxcallsuseRolloutRevisionsActionsProvider(rollout)withoutonError. Thecatchhandlers then consume patch errors throughonError?.(...), so header actions can fail silently. Pass an error reporter fromRolloutNavPage.tsxor rethrow when no reporter exists.🤖 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/hooks/useRolloutRevisionsActionsProvider.tsx` around lines 33 - 36, Update the revision-action error handling between useRolloutRevisionsActionsProvider and RolloutNavPage so failures from promoteRollout and the other action catch handlers always reach a user-visible reporter. Prefer passing RolloutNavPage’s existing error handler into useRolloutRevisionsActionsProvider; otherwise rethrow errors when onError is absent instead of silently consuming them.
🧹 Nitpick comments (1)
src/gitops/components/rollout/revisions/Revisions.tsx (1)
399-471: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd tests for the new revision action states and failures.
Cover Promote, Full Promote, Abort, Retry, and Restart. Test both disabled-state conditions and rejected patch requests. The supplied Codecov report shows very low patch coverage and should be refreshed against the current base branch.
🤖 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 399 - 471, Add or update tests covering the Promote, Full Promote, Abort, Retry, and Restart actions in the Revisions component. Verify each action’s disabled-state behavior, including the phase and deployment conditions, and assert that rejected action requests invoke onRevisionError with the appropriate action label. Refresh the Codecov report against the current base branch.
🤖 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 `@package.json`:
- Line 137: Add a trailing newline after the final closing brace in package.json
so the file satisfies Prettier formatting and lint checks.
---
Outside diff comments:
In `@src/gitops/components/rollout/hooks/useRolloutRevisionsActionsProvider.tsx`:
- Around line 33-36: Update the revision-action error handling between
useRolloutRevisionsActionsProvider and RolloutNavPage so failures from
promoteRollout and the other action catch handlers always reach a user-visible
reporter. Prefer passing RolloutNavPage’s existing error handler into
useRolloutRevisionsActionsProvider; otherwise rethrow errors when onError is
absent instead of silently consuming them.
---
Nitpick comments:
In `@src/gitops/components/rollout/revisions/Revisions.tsx`:
- Around line 399-471: Add or update tests covering the Promote, Full Promote,
Abort, Retry, and Restart actions in the Revisions component. Verify each
action’s disabled-state behavior, including the phase and deployment conditions,
and assert that rejected action requests invoke onRevisionError with the
appropriate action label. Refresh the Codecov report against the current base
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: e7f487c7-b2bc-4787-bbee-e8204d75b480
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
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/RolloutList.tsxsrc/gitops/components/rollout/RolloutNavPage.tsxsrc/gitops/components/rollout/hooks/useRolloutRevisionsActionsProvider.tsxsrc/gitops/components/rollout/revisions/Revisions.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/gitops/components/rollout/RolloutNavPage.tsx
Signed-off-by: Keith Chong <kykchong@redhat.com>
Signed-off-by: Keith Chong <kykchong@redhat.com>
See [GITOPS-8568] (https://redhat.atlassian.net/browse/GITOPS-8568)