[ci] Refactor Github workflows - #1107
Conversation
|
Warning Rate limit exceededAndrei Kvapil (@kvaps) has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 35 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (7)
""" WalkthroughThe pull request refactors GitHub Actions workflows by drastically simplifying the release workflow to only keep the final tagging and release job, removing all intermediate setup and testing jobs. The main pull request workflow is updated to support release PRs, introduces dynamic test matrix generation, and refactors artifact fetching. Additionally, end-to-end Bats test scripts for application provisioning and cluster provisioning are deleted, and a new tenant creation test is added to the Cozystack installation tests. Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request Event
participant GH as GitHub Actions
participant Runner as Self-hosted Runner
PR->>GH: PR closed (with "release" label)
GH->>Runner: Run finalize job
Runner->>GH: Checkout repo, extract tag, update branch, publish release
sequenceDiagram
participant PR as Pull Request Event
participant GH as GitHub Actions
participant Install as install_cozystack
participant Matrix as detect_test_matrix
participant Test as test_apps
participant Cleanup as cleanup
PR->>GH: PR opened/updated
GH->>Install: Fetch artifacts (build or draft release), prepare environment
GH->>Matrix: Generate dynamic test matrix
Install->>Test: Provide environment
Matrix->>Test: Provide test matrix
Test->>Cleanup: Run tests and cleanup
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
bb6a1f8 to
fde3fd3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/pull-requests.yaml (2)
156-156: Fix trailing spaces.Remove trailing whitespace to comply with YAML formatting standards.
- +
212-212: Fix comma spacing in YAML.Add proper spacing after the comma to improve readability and comply with YAML formatting standards.
- needs: [install_cozystack,detect_test_matrix] + needs: [install_cozystack, detect_test_matrix]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/pull-requests-release.yaml(1 hunks).github/workflows/pull-requests.yaml(4 hunks)hack/e2e-apps.bats(0 hunks)hack/e2e-apps/tenant.bats(0 hunks)hack/e2e-cluster.bats(1 hunks)packages/core/testing/Makefile(0 hunks)
💤 Files with no reviewable changes (3)
- hack/e2e-apps/tenant.bats
- packages/core/testing/Makefile
- hack/e2e-apps.bats
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/pull-requests.yaml
[error] 156-156: trailing spaces
(trailing-spaces)
[warning] 212-212: too few spaces after comma
(commas)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
🔇 Additional comments (8)
hack/e2e-cluster.bats (1)
393-412: Well-structured tenant creation test.The new test case follows good testing practices with proper conditional creation, appropriate wait conditions, and clear resource specifications. The isolated tenant configuration aligns with the test case name and objectives.
.github/workflows/pull-requests-release.yaml (2)
1-5: Workflow simplification improves maintainability.The refactoring to focus solely on the final release step reduces complexity and potential failure points. The trigger restriction to only closed events is appropriate for a release-focused workflow.
7-10: Concurrency group naming is consistent.The shortened concurrency group name maintains the same functionality while being more concise, consistent with the main PR workflow changes.
.github/workflows/pull-requests.yaml (5)
9-9: Concurrency group naming is consistent.The shortened concurrency group name aligns with the release workflow changes and maintains the same functionality.
60-66: Job consolidation improves workflow efficiency.The merge of environment preparation into the
install_cozystackjob reduces complexity and eliminates unnecessary job dependencies. The explicit permissions are a good security practice.
78-92: Conditional artifact download logic is well-structured.The separation between regular PR and release PR artifact handling is clear and appropriate. The path specifications ensure artifacts are placed in the correct directory structure.
193-206: Dynamic test matrix generation is an improvement.The dynamic detection of test files is more maintainable than static lists and will automatically include new test files as they're added.
93-156: Complex release PR asset fetching needs verification.The GitHub API integration for fetching draft release assets is sophisticated but adds complexity. Ensure that error handling covers edge cases like missing assets or API failures.
#!/bin/bash # Description: Verify GitHub API patterns and error handling in workflow scripts # Search for similar GitHub API usage patterns in the codebase rg -A 5 -B 5 "github\.rest\.repos\.listReleases" # Check for error handling patterns around API calls rg -A 10 "core\.setFailed.*Draft release.*not found"
fde3fd3 to
1c5a302
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/workflows/pull-requests.yaml (4)
7-9: Fix formatting issuesStatic analysis detected trailing spaces and formatting issues.
Apply this fix:
-# Cancel in‑flight runs for the same PR when a new push arrives. +# Cancel in-flight runs for the same PR when a new push arrives.
153-153: Fix trailing spacesStatic analysis detected trailing spaces.
Remove the trailing spaces at the end of line 153.
237-237: Fix trailing spacesStatic analysis detected trailing spaces.
Remove the trailing spaces at the end of line 237.
291-294: Fix comma spacingStatic analysis detected insufficient spacing after comma.
Apply this fix:
- needs: [install_cozystack,detect_test_matrix] + needs: [install_cozystack, detect_test_matrix]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/pull-requests-release.yaml(1 hunks).github/workflows/pull-requests.yaml(3 hunks)hack/e2e-apps.bats(0 hunks)hack/e2e-apps/tenant.bats(0 hunks)hack/e2e-cluster.bats(0 hunks)hack/e2e-install-cozystack.bats(3 hunks)packages/core/testing/Makefile(0 hunks)
💤 Files with no reviewable changes (4)
- packages/core/testing/Makefile
- hack/e2e-apps/tenant.bats
- hack/e2e-apps.bats
- hack/e2e-cluster.bats
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/pull-requests-release.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/pull-requests.yaml
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 237-237: trailing spaces
(trailing-spaces)
[warning] 294-294: too few spaces after comma
(commas)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
🔇 Additional comments (5)
hack/e2e-install-cozystack.bats (3)
23-25: LGTM: Improved HelmRelease filtering for system applicationsThe addition of the
cozystack.io/system-app=truelabel filter makes the tests more focused and targeted by only checking system applications. This reduces noise and improves test reliability.
45-49: LGTM: Defensive programming for storage pool creationThe logic to check for existing storage pools before creation is a good defensive programming practice that prevents errors when the script is run multiple times. The implementation correctly parses the LINSTOR output to identify existing pools.
163-182: LGTM: Consolidated tenant creation testThis new test case effectively consolidates tenant creation testing that was previously in separate scripts. The test properly creates a tenant with isolated mode and waits for the appropriate conditions.
.github/workflows/pull-requests.yaml (2)
275-287: LGTM: Dynamic test matrix generationThe new
detect_test_matrixjob that dynamically generates the test matrix by scanning e2e app scripts is a significant improvement over static matrices. This makes the workflow more maintainable and automatically adapts to new test files.
86-99: Verify branch name pattern matching for release PRsThe regex pattern
^release-(\d+\.\d+\.\d+(?:[-\w\.]+)?)$should handle various release branch formats correctly, but ensure it covers all expected patterns used in your repository.#!/bin/bash # Check recent release branches to verify the pattern matches correctly git branch -r | grep -E 'origin/release-' | head -10
405bbe9 to
4242973
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/pull-requests.yaml (1)
169-235: Significant code duplication with prepare_env job.The workspace preparation logic (lines 200-229) is duplicated almost identically in both
prepare_envandinstall_cozystackjobs. This violates the DRY principle and makes maintenance difficult.Consider extracting the common workspace preparation steps into a reusable composite action or consolidating the logic. The duplication includes:
- Sandbox ID generation
- Workspace copying
- Systemctl timer management
🧹 Nitpick comments (4)
.github/workflows/pull-requests.yaml (4)
65-66: Fix YAML formatting issues.Static analysis detected formatting issues in the outputs section.
Apply this diff to fix the indentation and spacing:
outputs: - installer_id: ${{ steps.fetch_assets.outputs.installer_id }} - disk_id: ${{ steps.fetch_assets.outputs.disk_id }} + installer_id: ${{ steps.fetch_assets.outputs.installer_id }} + disk_id: ${{ steps.fetch_assets.outputs.disk_id }}
163-163: Remove trailing spaces.Static analysis detected trailing spaces.
Apply this diff to remove trailing spaces:
- --unit=rm-workspace-$SANDBOX_NAME \ + --unit=rm-workspace-$SANDBOX_NAME \
198-198: Remove trailing spaces.Static analysis detected trailing spaces.
Apply this diff to remove trailing spaces:
- env: + env:
255-255: Fix comma spacing.Static analysis detected missing space after comma.
Apply this diff to fix the spacing:
- needs: [install_cozystack,detect_test_matrix] + needs: [install_cozystack, detect_test_matrix]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/pull-requests-release.yaml(1 hunks).github/workflows/pull-requests.yaml(3 hunks)hack/e2e-apps.bats(0 hunks)hack/e2e-apps/tenant.bats(0 hunks)hack/e2e-cluster.bats(0 hunks)hack/e2e-install-cozystack.bats(3 hunks)packages/core/testing/Makefile(0 hunks)
💤 Files with no reviewable changes (4)
- packages/core/testing/Makefile
- hack/e2e-apps/tenant.bats
- hack/e2e-apps.bats
- hack/e2e-cluster.bats
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/pull-requests.yaml
[warning] 65-65: wrong indentation: expected 6 but found 5
(indentation)
[warning] 66-66: too many spaces after colon
(colons)
[error] 163-163: trailing spaces
(trailing-spaces)
[error] 198-198: trailing spaces
(trailing-spaces)
[warning] 255-255: too few spaces after comma
(commas)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
🔇 Additional comments (11)
hack/e2e-install-cozystack.bats (3)
23-25: LGTM! Improved HelmRelease filtering.The addition of the
cozystack.io/system-app=truelabel filter appropriately narrows the scope to only wait for system applications, which is more precise than waiting for all HelmReleases.
45-49: LGTM! Added idempotent storage pool creation.The logic to check for existing storage pools before creation prevents errors on test re-runs and makes the test more robust.
163-182: LGTM! Added tenant creation test coverage.The new test appropriately covers tenant creation with isolated mode enabled, which aligns with the removal of more comprehensive tenant tests mentioned in the AI summary.
.github/workflows/pull-requests-release.yaml (3)
1-1: LGTM! Workflow name reflects its purpose.The rename from "Releasing PR" to "Pull Request & Release" better describes the workflow's dual purpose.
5-5: LGTM! Appropriate trigger for release workflow.Changing the trigger to only
closedevents is correct for a workflow that should only run when a PR is merged, not on every push.
9-9: LGTM! Consistent concurrency group naming.The shortened concurrency group name
pr-is consistent with the other workflow and more concise..github/workflows/pull-requests.yaml (5)
9-9: LGTM! Consistent concurrency group naming.The shortened concurrency group name
pr-is consistent with the release workflow and more concise.
60-117: LGTM! Well-structured asset resolution for release PRs.The new
resolve_assetsjob appropriately handles asset resolution for release PRs by:
- Extracting tags from branch names with proper validation
- Fetching draft release assets via GitHub API
- Providing clear error messages for missing assets
119-168: LGTM! Unified environment preparation logic.The refactored
prepare_envjob effectively consolidates environment setup with proper conditional logic for regular vs. release PRs.
236-248: LGTM! Dynamic test matrix generation.The new
detect_test_matrixjob appropriately generates a dynamic test matrix by scanning for e2e test scripts, which is more maintainable than hardcoded matrices.
252-252: LGTM! Dynamic matrix usage.The matrix now properly uses the dynamically generated output from
detect_test_matrix, improving maintainability.
9a069de to
4f76706
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/pull-requests.yaml (1)
149-169: Significant code duplication remains between jobs.Despite the past review comment being marked as addressed, substantial duplication still exists between
prepare_envandinstall_cozystackjobs, including:
- Identical sandbox setup logic (lines 149-169 vs 202-231)
- Similar workspace preparation steps
- Duplicate systemctl cleanup commands
This violates DRY principles and increases maintenance burden.
Consider extracting the common sandbox setup logic into a reusable composite action or consolidating these jobs where possible. The duplication includes ~20 lines of identical workspace management code.
Also applies to: 202-231
🧹 Nitpick comments (3)
.github/workflows/pull-requests.yaml (3)
65-66: Fix YAML formatting issues.The static analysis tools have identified formatting problems that should be corrected for consistency.
Apply this diff to fix the indentation and spacing issues:
- installer_id: ${{ steps.fetch_assets.outputs.installer_id }} - disk_id: ${{ steps.fetch_assets.outputs.disk_id }} + installer_id: ${{ steps.fetch_assets.outputs.installer_id }} + disk_id: ${{ steps.fetch_assets.outputs.disk_id }}
164-164: Remove trailing spaces.Static analysis detected trailing spaces that should be removed for consistency.
Also applies to: 200-200
258-258: Fix spacing after comma.Add a space after the comma for consistent formatting.
- needs: [install_cozystack,detect_test_matrix] + needs: [install_cozystack, detect_test_matrix]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/pull-requests-release.yaml(1 hunks).github/workflows/pull-requests.yaml(4 hunks)hack/e2e-apps.bats(0 hunks)hack/e2e-apps/tenant.bats(0 hunks)hack/e2e-cluster.bats(0 hunks)hack/e2e-install-cozystack.bats(3 hunks)packages/core/testing/Makefile(0 hunks)
💤 Files with no reviewable changes (4)
- hack/e2e-apps/tenant.bats
- packages/core/testing/Makefile
- hack/e2e-apps.bats
- hack/e2e-cluster.bats
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/pull-requests-release.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/pull-requests.yaml
[warning] 65-65: wrong indentation: expected 6 but found 5
(indentation)
[warning] 66-66: too many spaces after colon
(colons)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 200-200: trailing spaces
(trailing-spaces)
[warning] 258-258: too few spaces after comma
(commas)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
🔇 Additional comments (5)
hack/e2e-install-cozystack.bats (3)
23-25: Good optimization - filtering to system apps only.The filtering logic using
cozystack.io/system-app=trueimproves efficiency by focusing readiness checks only on system applications rather than all HelmReleases.
45-49: Smart addition - prevents duplicate storage pool creation.The logic to check for existing "data" storage pools before creation prevents errors and makes the test idempotent, which is a best practice for e2e tests.
163-182: Good test coverage addition.The new tenant creation test with isolated mode enabled adds valuable test coverage for tenant provisioning functionality. The test properly waits for both the HelmRelease and namespace to be ready.
.github/workflows/pull-requests.yaml (2)
60-117: Well-structured asset resolution for release PRs.The new
resolve_assetsjob properly handles tag extraction from release branches and fetches asset IDs from draft releases. The error handling and validation logic is appropriate.
238-250: Excellent dynamic test matrix approach.The dynamic test matrix generation by scanning e2e app scripts is a significant improvement over static matrices, providing better maintainability and automatic discovery of new tests.
4f76706 to
1b347b8
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/pull-requests.yaml (1)
161-164: Trailing whitespace – fails YAML-lint gatingLines flagged by YAML-lint contain trailing spaces.
While harmless to the runner, the repo’s lint gate will fail; please strip them.Also applies to: 195-200
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/pull-requests-release.yaml(1 hunks).github/workflows/pull-requests.yaml(4 hunks)hack/e2e-apps.bats(0 hunks)hack/e2e-apps/tenant.bats(0 hunks)hack/e2e-cluster.bats(0 hunks)hack/e2e-install-cozystack.bats(3 hunks)packages/core/testing/Makefile(0 hunks)
💤 Files with no reviewable changes (4)
- packages/core/testing/Makefile
- hack/e2e-apps/tenant.bats
- hack/e2e-apps.bats
- hack/e2e-cluster.bats
🚧 Files skipped from review as they are similar to previous changes (1)
- hack/e2e-install-cozystack.bats
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/pull-requests.yaml
[warning] 65-65: wrong indentation: expected 6 but found 5
(indentation)
[warning] 66-66: too many spaces after colon
(colons)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 200-200: trailing spaces
(trailing-spaces)
[warning] 257-257: too few spaces after comma
(commas)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
🔇 Additional comments (1)
.github/workflows/pull-requests-release.yaml (1)
20-22: Label-gated release trigger looks goodThe additional
merged == true && contains(label == 'release')guard is a clean way to ensure the job fires only for intentional release PRs.
1b347b8 to
6c5f0f5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/pull-requests.yaml (2)
64-66: Incorrect indentation still breaks theoutputsblock
YAML-lint is flagging lines 65-66 (expected 6 but found 5).
GitHub will refuse to parse the workflow at run-time.- outputs: - installer_id: ${{ steps.fetch_assets.outputs.installer_id }} - disk_id: ${{ steps.fetch_assets.outputs.disk_id }} + outputs: + installer_id: ${{ steps.fetch_assets.outputs.installer_id }} + disk_id: ${{ steps.fetch_assets.outputs.disk_id }}
138-146:_out/assetsdirectory missing beforecurldownload (release PR path)
curlwill fail because the parent directory does not exist when the job is running on a release PR (no artefact-download step precedes it).- - name: Download assets from draft release (release PR) + - name: Prepare _out/assets && download assets (release PR) if: contains(github.event.pull_request.labels.*.name, 'release') run: | + mkdir -p _out/assets curl -sSL -H "Authorization: token ${GH_PAT}" -H "Accept: application/octet-stream" \ -o _out/assets/nocloud-amd64.raw.xz \ "https://api.github.com/repos/${GITHUB_REPOSITORY}/releases/assets/${{ needs.resolve_assets.outputs.disk_id }}"
🧹 Nitpick comments (3)
.github/workflows/pull-requests.yaml (3)
70-78: Redundant step-levelif:checks
The entireresolve_assetsjob already executes only when the PR has thereleaselabel (job.if). Repeating the same condition on every step is noise and invites desynchronisation. Remove the duplicateif:clauses to keep the workflow concise.
160-166: Trailing whitespace sneaked in
Lines 164 and 200 contain trailing spaces, which YAML-lint reports as errors. Please strip them to keep the workflow lint-clean.Also applies to: 195-200
150-160: Workspace preparation duplicated across two jobs
prepare_envandinstall_cozystackrepeat the same 30-line block for workspace copy & timer scheduling. Consider moving this into:
- A small bash script committed to the repo and invoked from both jobs, or
- A local reusable step (
uses: ./.github/actions/prepare-workspace).This will DRY the workflow and reduce maintenance burden.
Also applies to: 205-214
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/pull-requests-release.yaml(1 hunks).github/workflows/pull-requests.yaml(4 hunks)hack/e2e-apps.bats(0 hunks)hack/e2e-apps/tenant.bats(0 hunks)hack/e2e-cluster.bats(0 hunks)hack/e2e-install-cozystack.bats(3 hunks)packages/core/testing/Makefile(0 hunks)
💤 Files with no reviewable changes (4)
- packages/core/testing/Makefile
- hack/e2e-apps/tenant.bats
- hack/e2e-apps.bats
- hack/e2e-cluster.bats
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/pull-requests-release.yaml
- hack/e2e-install-cozystack.bats
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/pull-requests.yaml
[warning] 65-65: wrong indentation: expected 6 but found 5
(indentation)
[warning] 66-66: too many spaces after colon
(colons)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 200-200: trailing spaces
(trailing-spaces)
[warning] 257-257: too few spaces after comma
(commas)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
64e722a to
56dc0e8
Compare
Signed-off-by: Andrei Kvapil <[email protected]>
56dc0e8 to
e7681de
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/pull-requests.yaml (2)
64-66: Indentation underoutputsbreaks workflow parsing
installer_id/disk_idare indented 5 spaces instead of the required 6, so the
YAML is syntactically invalid and the workflow will be rejected at compile time.- outputs: - installer_id: ${{ steps.fetch_assets.outputs.installer_id }} - disk_id: ${{ steps.fetch_assets.outputs.disk_id }} + outputs: + installer_id: ${{ steps.fetch_assets.outputs.installer_id }} + disk_id: ${{ steps.fetch_assets.outputs.disk_id }}
138-146:_out/assetsdirectory may not exist in release pathFor release PRs the artefact-download step skips
actions/download-artifact, so
the_out/assetsdirectory is never created.
curl -o _out/assets/nocloud-amd64.raw.xz …will therefore fail.Add an explicit mkdir before the curl:
- - name: Download assets from draft release (release PR) + - name: Prepare _out/assets && download Talos image (release PR) if: contains(github.event.pull_request.labels.*.name, 'release') - run: | - curl -sSL -H "Authorization: token ${GH_PAT}" -H "Accept: application/octet-stream" \ - -o _out/assets/nocloud-amd64.raw.xz \ - "https://api.github.com/repos/${GITHUB_REPOSITORY}/releases/assets/${{ needs.resolve_assets.outputs.disk_id }}" + run: | + mkdir -p _out/assets + curl -sSL -H "Authorization: token ${GH_PAT}" -H "Accept: application/octet-stream" \ + -o _out/assets/nocloud-amd64.raw.xz \ + "https://api.github.com/repos/${GITHUB_REPOSITORY}/releases/assets/${{ needs.resolve_assets.outputs.disk_id }}"
🧹 Nitpick comments (4)
.github/workflows/pull-requests-release.yaml (1)
40-50: Update the tag command or the comment – they contradict each otherThe comment says “create / push annotated tag” but the command uses
git tag -f …which produces a light-weight tag (no annotation).
Either add the-aflag (plus-mfor the message) or adjust the comment.- git tag -f ${{ steps.get_tag.outputs.tag }} ${{ github.sha }} - git push -f origin ${{ steps.get_tag.outputs.tag }} + git tag -fa ${{ steps.get_tag.outputs.tag }} -m "Release ${{ steps.get_tag.outputs.tag }}" ${{ github.sha }} + git push -f origin ${{ steps.get_tag.outputs.tag }}.github/workflows/pull-requests.yaml (3)
180-200: Trailing whitespace – GitHub rejects YAML with strict lintersLine 200 ends with stray spaces. Remove them to keep the file linter-clean.
210-223: Empty matrix ⇒ entire job skipped ⇒ pipeline marked as failedIf
hack/e2e-appscontains no.batsfiles,appsbecomes[].
An empty matrix causes “no jobs were found”, which GitHub treats as a
workflow error. Consider defaulting to a dummy value or gating the job.Example guard:
detect_test_matrix: … outputs: matrix: ${{ steps.set.outputs.matrix }} has_apps: ${{ steps.set.outputs.has_apps }} # in the generating script apps_json=$( … ) echo "matrix={\"app\":$apps_json}" >> "$GITHUB_OUTPUT" echo "has_apps=$([[ $apps_json != \"[]\" ]])" >> "$GITHUB_OUTPUT" # and in test_apps job if: ${{ needs.detect_test_matrix.outputs.has_apps == 'true' }}
229-230: Minor style: add a space after the comma in theneedslist- needs: [install_cozystack,detect_test_matrix] + needs: [install_cozystack, detect_test_matrix]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/pull-requests-release.yaml(1 hunks).github/workflows/pull-requests.yaml(4 hunks)hack/e2e-apps.bats(0 hunks)hack/e2e-apps/tenant.bats(0 hunks)hack/e2e-cluster.bats(0 hunks)hack/e2e-install-cozystack.bats(3 hunks)packages/core/testing/Makefile(1 hunks)
💤 Files with no reviewable changes (3)
- hack/e2e-apps/tenant.bats
- hack/e2e-apps.bats
- hack/e2e-cluster.bats
🚧 Files skipped from review as they are similar to previous changes (2)
- hack/e2e-install-cozystack.bats
- packages/core/testing/Makefile
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/pull-requests.yaml
[warning] 65-65: wrong indentation: expected 6 but found 5
(indentation)
[warning] 66-66: too many spaces after colon
(colons)
[error] 200-200: trailing spaces
(trailing-spaces)
[warning] 229-229: too few spaces after comma
(commas)
Signed-off-by: Andrei Kvapil <[email protected]> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> - **New Features** - Pull request workflows now support release pull requests by fetching artifacts from draft releases and running all jobs without label-based exclusions. - Test matrices are now generated dynamically, improving flexibility in end-to-end application testing. - Added a new end-to-end test verifying tenant creation with isolated mode enabled. - **Refactor** - Workflow steps and job dependencies have been streamlined for improved efficiency and maintainability. - Workflow names and concurrency group names have been updated for clarity. - Environment preparation and artifact handling have been unified into consolidated jobs. - Release-related workflow simplified to a single finalize job. - Makefile targets for asset copying and test execution have been reorganized for better modularity. - **Tests** - End-to-end application and cluster test scripts have been removed. - Removed collective end-to-end test target; individual app test targets remain. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Andrei Kvapil <[email protected]>
1 participant
Signed-off-by: Andrei Kvapil [email protected]
Summary by CodeRabbit
New Features
Refactor
Tests