Remove ci connected mode#334
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughGitHub Actions workflows consolidate E2E deployment testing to use disconnected-mode execution by default. The primary e2e-deployment.yml workflow removes run-connected and run-disconnected input parameters, simplifies decision logic and outputs, and gates the disconnected job on a unified should_run output. All downstream callers (RC workflow, scheduled workflow, slash commands) are updated to invoke the simplified workflow without the removed mode flags. Documentation and help text reflect the new disconnected-first approach. ChangesE2E Deployment Mode Consolidation
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Tarball created: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/e2e-deployment.yml (1)
3-8:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale header comment after removing connected mode.
Header still references "Both jobs" and "Manual dispatch allows selecting which modes to run". Only the disconnected job remains, and there are no mode-selection inputs.
📝 Proposed text update
# Unified end-to-end deployment testing for disconnected mode. # # - e2e-disconnected: Full air-gapped deployment with local mirror registry # -# Both jobs run on every PR and nightly schedule. Manual dispatch allows -# selecting which modes to run, skipping cleanup, and sending Slack notifications. +# Runs on every PR and nightly schedule. Manual dispatch allows selecting the +# storage plugin, skipping cleanup, and sending Slack notifications.🤖 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 @.github/workflows/e2e-deployment.yml around lines 3 - 8, The header comment is stale: update it to describe that this workflow runs the single e2e-disconnected job (full air-gapped deployment with local mirror registry) on PRs and nightly schedules, and remove references to "Both jobs" and any mention of manual dispatch mode-selection since there are no mode-selection inputs; reference the existing job name e2e-disconnected and any manual_dispatch/input keys in the workflow and replace the header with concise text explaining the single-job purpose and trigger behaviors.
🧹 Nitpick comments (1)
.github/workflows/e2e-deployment.yml (1)
97-97: 💤 Low valueDrop stale reference to connected mode in inline comment.
The trailing sentence still mentions "Connected always uses lvms" even though the connected mode/job has been removed.
📝 Proposed text update
- # ODF only runs in disconnected mode. Connected always uses lvms. + # Default storage plugin is lvms; odf is selected only via manual dispatch + # or when ODF-specific paths change.🤖 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 @.github/workflows/e2e-deployment.yml at line 97, Update the inline comment that reads "ODF only runs in disconnected mode. Connected always uses lvms" to remove the stale "Connected always uses lvms" clause; leave a concise comment like "ODF only runs in disconnected mode." (Locate the inline comment containing the phrase "Connected always uses lvms" and replace it accordingly.)
🤖 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 @.github/workflows/e2e-deployment.yml:
- Around line 130-135: The job e2e-disconnected is being skipped because the if
condition includes "github.event_name != 'workflow_dispatch'"; remove that
event-name guard so the job's if only checks
needs.check-e2e-needed.outputs.should_run == 'true' (keep the existing
runs-on/matrix logic unchanged). Locate the e2e-disconnected job and update its
if expression to drop the "&& github.event_name != 'workflow_dispatch'" clause
so workflow_dispatch callers can run the job.
In @.github/workflows/slash-command.yml:
- Line 105: Update the help text row that documents the `/test all` slash
command by removing the stale suffix "(both modes)" so it accurately reflects
that `/test all` runs validation + tarball + infra + e2e (disconnected only);
locate the table entry containing the string "/test all" in the
.github/workflows/slash-command.yml and edit the description to "Run validation
+ tarball + infra + e2e (disconnected)" or simply "Run validation + tarball +
infra + e2e".
---
Outside diff comments:
In @.github/workflows/e2e-deployment.yml:
- Around line 3-8: The header comment is stale: update it to describe that this
workflow runs the single e2e-disconnected job (full air-gapped deployment with
local mirror registry) on PRs and nightly schedules, and remove references to
"Both jobs" and any mention of manual dispatch mode-selection since there are no
mode-selection inputs; reference the existing job name e2e-disconnected and any
manual_dispatch/input keys in the workflow and replace the header with concise
text explaining the single-job purpose and trigger behaviors.
---
Nitpick comments:
In @.github/workflows/e2e-deployment.yml:
- Line 97: Update the inline comment that reads "ODF only runs in disconnected
mode. Connected always uses lvms" to remove the stale "Connected always uses
lvms" clause; leave a concise comment like "ODF only runs in disconnected mode."
(Locate the inline comment containing the phrase "Connected always uses lvms"
and replace it accordingly.)
🪄 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
Run ID: e2505ea0-767d-4e4d-9ed6-fa3704210959
📒 Files selected for processing (5)
.github/workflows/e2e-deployment-rc.yml.github/workflows/e2e-deployment.yml.github/workflows/e2e-odf-schedule.yml.github/workflows/slash-command.ymldocs/CI_WORKFLOWS.md
💤 Files with no reviewable changes (3)
- docs/CI_WORKFLOWS.md
- .github/workflows/e2e-odf-schedule.yml
- .github/workflows/e2e-deployment-rc.yml
rporres
force-pushed
the
remove-connected-e2e
branch
from
5edf07d to
55420bb
Compare
|
Tarball created: |
rporres
force-pushed
the
remove-connected-e2e
branch
from
55420bb to
1feaf51
Compare
|
Tarball created: |
We initially added connected deployment for testing purposes. Back then, the disconnected job took 8/6 hours to run so we had to get some option to test something in our CI. Now if we don't have a use case for connected mode we can remove it. xref: https://redhat-internal.slack.com/archives/C09G9BS83T9/p1778134946053789 Signed-off-by: Rafa Porres Molina <rporresm@redhat.com>
rporres
force-pushed
the
remove-connected-e2e
branch
from
1feaf51 to
660fc3f
Compare
|
Tarball created: |
| steps: | ||
| - name: Trigger E2E (LVMS) | ||
| run: | | ||
| gh workflow run e2e-deployment.yml --ref 0-rc -f send-slack-notification=true |
There was a problem hiding this comment.
| gh workflow run e2e-deployment.yml --ref 0-rc \ | |
| -f run-connected=false \ | |
| -f run-disconnected=true \ | |
| -f storage-plugin=lvms \ | |
| -f send-slack-notification=true |
2 participants
We initially added connected deployment for testing purposes. Back then, the disconnected job took 8/6 hours to run so we had to get some option to test something in our CI. Now if we don't have a use case for connected mode we can remove it.
xref: https://redhat-internal.slack.com/archives/C09G9BS83T9/p1778134946053789
Summary by CodeRabbit
Chores
Documentation