Skip to content

[fournos-launcher] Test v0.4.z#53

Merged
kpouget merged 2 commits into
openshift-psap:mainfrom
kpouget:test
May 12, 2026
Merged

[fournos-launcher] Test v0.4.z#53
kpouget merged 2 commits into
openshift-psap:mainfrom
kpouget:test

Conversation

@kpouget
Copy link
Copy Markdown
Contributor

@kpouget kpouget commented May 5, 2026

Summary by CodeRabbit

  • Chores
    • Standardized how the workload namespace is read and set across CI tools used for resolving and exporting Fournos jobs, improving consistency in job resolution and status updates.
  • New Features
    • Added an automated CI workflow to build and push the Fournos container image to the registry on main branch updates, releases, and manual runs.

Review Change Stack

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 5, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign sjmonson for approval. For more information see the Code Review Process.

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

Details Needs approval from an approver in each of these files:

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

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 5, 2026

Warning

Rate limit exceeded

@kpouget has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 56 minutes and 20 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 91bb565c-7a48-4fc6-a12b-e80b48b1e345

📥 Commits

Reviewing files that changed from the base of the PR and between a6e222b and 2cf1ed9.

📒 Files selected for processing (1)
  • .github/workflows/image-push.yaml
📝 Walkthrough

Walkthrough

Migrates runtime code to read Fournos workload namespace from FOURNOS_WORKLOAD_NAMESPACE (replacing FOURNOS_NAMESPACE) and adds a GitHub Actions workflow to build and push the Fournos image to quay.io/rh_perfscale with event-based tagging.

Changes

Namespace Environment Variable Migration

Layer / File(s) Summary
Fetch Function Validation
projects/core/ci_entrypoint/fournos_resolve.py
fetch_fournos_job() reads namespace from FOURNOS_WORKLOAD_NAMESPACE and raises ValueError if missing.
CLI Option and Environment Binding
projects/core/ci_entrypoint/fournos_resolve.py
--namespace Click option help/envvar updated to FOURNOS_WORKLOAD_NAMESPACE; option handler sets os.environ["FOURNOS_WORKLOAD_NAMESPACE"].
Export Status Workflow
projects/core/library/export.py
_update_fjob_export_status() reads FOURNOS_WORKLOAD_NAMESPACE when building oc get/patch fjob/... commands.

CI Image Build & Push

Layer / File(s) Summary
Image build/push workflow
.github/workflows/image-push.yaml
New GitHub Actions workflow that builds the Fournos image with Buildah and pushes to quay.io/rh_perfscale, producing tags based on event type (commit SHA, latest for pushes, stable + release tag for releases).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • openshift-psap/forge#45: Earlier PR that renames the Fournos namespace environment variable and updates code reading it.

🐰 A small hop to tidy the name,
WORKLOAD whispers where jobs belong,
CI hums, builds an image true,
Tags by event — sha, latest, stable too,
Rabbit cheers: coherent config and song. 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title '[fournos-launcher] Test v0.4.z' is vague and generic, using 'Test' without describing the actual technical changes (namespace refactoring and CI workflow additions). Replace with a more descriptive title like '[fournos-launcher] Use FOURNOS_WORKLOAD_NAMESPACE instead of FOURNOS_NAMESPACE' to clearly communicate the main technical change.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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 and usage tips.

@kpouget kpouget changed the title [fourno] Test v0.4.0 [fournos-launcher] Test v0.4.0 May 5, 2026
@kpouget
Copy link
Copy Markdown
Contributor Author

kpouget commented May 11, 2026

/test fournos skeleton quick_test
/var fournos.namespace: psap-automation-wip
/cluster psap-mgmt

@psap-forge-bot
Copy link
Copy Markdown

🔴 Test of 'fournos_launcher submit' failed after 00 hours 01 minutes 11 seconds 🔴

• Link to the test results.

• No reports generated...

Test configuration:

/test fournos skeleton quick_test
/var fournos.namespace: psap-automation-wip
/cluster psap-mgmt

• Failure indicator: Empty.
Execution logs

@kpouget
Copy link
Copy Markdown
Contributor Author

kpouget commented May 11, 2026

/test fournos skeleton quick_test
/var fournos.namespace: psap-automation-wip
/cluster psap-mgmt

@psap-forge-bot
Copy link
Copy Markdown

🟢 Test of 'skeleton test' succeeded after 00 hours 00 minutes 10 seconds 🟢

• Link to the test results.

• Generated 2 Caliper report(s):

  • summary_table.html
  • throughput_chart.html

Test configuration:

ci_job.cluster: psap-mgmt
ci_job.exclusive: true
ci_job.fjob: forge-skeleton-20260511-112523
ci_job.hardware:
  gpuCount: 4
  gpuType: h200
ci_job.name: skeleton quick_test
ci_job.owner: kpouget
project.args:
- quick_test
project.name: skeleton

Execution logs

@psap-forge-bot
Copy link
Copy Markdown

🟢 Test of 'fournos_launcher submit' succeeded after 00 hours 01 minutes 28 seconds 🟢

• Link to the test results.

• No reports generated...

Test configuration:

/test fournos skeleton quick_test
/var fournos.namespace: psap-automation-wip
/cluster psap-mgmt

Execution logs

@kpouget kpouget changed the title [fournos-launcher] Test v0.4.0 [fournos-launcher] Test v0.4.z May 12, 2026
@kpouget
Copy link
Copy Markdown
Contributor Author

kpouget commented May 12, 2026

merging this to get the forge image in quay.io

@kpouget kpouget merged commit b2cc2a6 into openshift-psap:main May 12, 2026
4 checks passed
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/workflows/image-push.yaml (1)

13-13: ⚡ Quick win

Pin GitHub Actions to commit SHAs for supply-chain hardening.

Lines 13, 29, and 38 use floating major tags (@v4, @v2). GitHub recommends pinning to immutable commit SHAs; tags can be moved or deleted and are not the only way to ensure an action is truly immutable.

Suggested fix pattern
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@<commit-sha>

-        uses: redhat-actions/buildah-build@v2
+        uses: redhat-actions/buildah-build@<commit-sha>

-        uses: redhat-actions/push-to-registry@v2
+        uses: redhat-actions/push-to-registry@<commit-sha>
🤖 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/image-push.yaml at line 13, Replace floating action tags
with immutable commit SHAs: locate the three "uses:" entries (e.g., "uses:
actions/checkout@v4", "uses: actions/setup-node@v2", "uses:
docker/build-push-action@v2") and update each tag to the corresponding full
commit SHA from the action's upstream GitHub repo (replace `@vX` with
@<commit-sha>). Fetch the appropriate SHA by opening the action repository and
copying the commit hash for the release/tag you want to pin, then commit those
SHA-pinned references so the workflow is immutably pinned.
🤖 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/image-push.yaml:
- Around line 10-11: The image-push job can race and overwrite the `latest` tag;
add GitHub Actions concurrency to the job to prevent stale pushes by defining a
concurrency group and enabling cancel-in-progress for the image-push job (use
the job name image-push as reference), e.g., set concurrency.group to a stable
identifier per branch or ref (such as github.ref or github.ref_name) and
concurrency.cancel-in-progress to true so an older run is cancelled when a newer
run for the same branch starts.
- Around line 22-24: The workflow currently appends
github.event.release.tag_name directly to TAGS which can include invalid
characters (e.g. '/') for OCI/Docker image tags; compute a sanitized tag (e.g.
CLEAN_TAG) from github.event.release.tag_name by replacing slashes with hyphens
and stripping any characters not in the allowed set [A-Za-z0-9_.-], then append
CLEAN_TAG to TAGS instead of the raw github.event.release.tag_name; update the
block that sets TAGS (the lines referencing TAGS and
github.event.release.tag_name) to use the sanitized variable.

---

Nitpick comments:
In @.github/workflows/image-push.yaml:
- Line 13: Replace floating action tags with immutable commit SHAs: locate the
three "uses:" entries (e.g., "uses: actions/checkout@v4", "uses:
actions/setup-node@v2", "uses: docker/build-push-action@v2") and update each tag
to the corresponding full commit SHA from the action's upstream GitHub repo
(replace `@vX` with @<commit-sha>). Fetch the appropriate SHA by opening the
action repository and copying the commit hash for the release/tag you want to
pin, then commit those SHA-pinned references so the workflow is immutably
pinned.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: c47d0ffb-1277-4742-9812-3aeb9e1a34a0

📥 Commits

Reviewing files that changed from the base of the PR and between ef5545a and a6e222b.

📒 Files selected for processing (3)
  • .github/workflows/image-push.yaml
  • projects/core/ci_entrypoint/fournos_resolve.py
  • projects/core/library/export.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • projects/core/library/export.py
  • projects/core/ci_entrypoint/fournos_resolve.py

Comment on lines +10 to +11
image-push:
runs-on: ubuntu-latest
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add concurrency control to prevent stale latest pushes.

On Line 10, runs are unconstrained; two close pushes to main can race and an older run may finish later and overwrite latest.

Suggested fix
 name: Image Push
+concurrency:
+  group: image-push-${{ github.ref }}
+  cancel-in-progress: true
 on:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
image-push:
runs-on: ubuntu-latest
name: Image Push
concurrency:
group: image-push-${{ github.ref }}
cancel-in-progress: true
on:
...
🤖 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/image-push.yaml around lines 10 - 11, The image-push job
can race and overwrite the `latest` tag; add GitHub Actions concurrency to the
job to prevent stale pushes by defining a concurrency group and enabling
cancel-in-progress for the image-push job (use the job name image-push as
reference), e.g., set concurrency.group to a stable identifier per branch or ref
(such as github.ref or github.ref_name) and concurrency.cancel-in-progress to
true so an older run is cancelled when a newer run for the same branch starts.

Comment on lines +22 to +24
if [[ "${{ github.event_name }}" == "release" ]]; then
TAGS="stable ${TAGS} ${{ github.event.release.tag_name }}"
fi
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Sanitize release tag before using it as an image tag.

On Line 23, ${{ github.event.release.tag_name }} is used directly; release tags may contain invalid container-tag chars (like /), causing build/push failures.

Suggested fix
       - name: Determine tags
         id: tags
         run: |
           TAGS="$(git rev-parse --short HEAD)"
           if [[ "${{ github.event_name }}" == "push" ]]; then
             TAGS="latest ${TAGS}"
           fi
           if [[ "${{ github.event_name }}" == "release" ]]; then
-            TAGS="stable ${TAGS} ${{ github.event.release.tag_name }}"
+            RELEASE_TAG="${{ github.event.release.tag_name }}"
+            SAFE_RELEASE_TAG="$(echo "${RELEASE_TAG}" | tr '/:@ ' '-' | tr -cd '[:alnum:]_.-')"
+            TAGS="stable ${TAGS} ${SAFE_RELEASE_TAG}"
           fi
           echo "tags=${TAGS}" >> "$GITHUB_OUTPUT"
What characters are allowed in OCI/Docker image tags, and is `/` allowed in a tag value?
🤖 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/image-push.yaml around lines 22 - 24, The workflow
currently appends github.event.release.tag_name directly to TAGS which can
include invalid characters (e.g. '/') for OCI/Docker image tags; compute a
sanitized tag (e.g. CLEAN_TAG) from github.event.release.tag_name by replacing
slashes with hyphens and stripping any characters not in the allowed set
[A-Za-z0-9_.-], then append CLEAN_TAG to TAGS instead of the raw
github.event.release.tag_name; update the block that sets TAGS (the lines
referencing TAGS and github.event.release.tag_name) to use the sanitized
variable.

@kpouget kpouget deleted the test branch May 12, 2026 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant