Skip to content

Fix the PR image comment job - #583

Open
leandroberetta wants to merge 1 commit into
netobserv:mainfrom
leandroberetta:fix-pr-image-comment
Open

leandroberetta wants to merge 1 commit into
netobserv:mainfrom
leandroberetta:fix-pr-image-comment

Conversation

@leandroberetta

Copy link
Copy Markdown
Member

Description

The comment on PR job of push_image_pr.yml has been failing since #12fc3df7 (Secure push job wrt branch interpolation) split the workflow into separate jobs:

ReferenceError: fs is not defined

Two things were lost in that split: the var fs = require('fs') line, and the artifact download step — ./commands-url lives in the pr artifact, which only the push job fetches.

The effect is that the image is built and pushed fine, but no comment is posted, so PR authors and QE never learn the image tag. On #552 that led to QE running the PR's plugin script against quay.io/netobserv/network-observability-cli:main (the default in commands/netobserv), whose binary does not have the new resolve-tls subcommand, and the collector pod failed with unknown command "resolve-tls".

Fix: read the URL in the push job, which already has the artifact, and pass it to the comment job as a job output, in the same env-var style the rest of that script uses.

Dependencies

n/a

Checklist

  • Does the changes in PR need specific configuration or environment set up for testing?
    • if so please describe it in PR description.
  • I have added thorough unit tests for the change.
  • QE requirements (check 1 from the list):
    • Standard QE validation, with pre-merge tests unless stated otherwise.
    • Regression tests only (e.g. refactoring with no user-facing change).
    • No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

Note that this workflow can only be exercised by a real ok-to-test label event, so the first labelled PR after this merges is the actual verification.

🤖 Generated with Claude Code

Since 12fc3df split the workflow into separate jobs, the comment job
calls fs.readFileSync() without requiring 'fs' and no longer downloads
the artifact holding ./commands-url, so it fails with "ReferenceError:
fs is not defined". The image is pushed but no comment is posted, so PR
authors and QE never get the image tag and fall back to :main.

Read the URL in the push job, which already has the artifact, and pass
it to the comment job as an output.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@openshift-ci

openshift-ci Bot commented Sep 15, 2026

Copy link
Copy Markdown

[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 stleerh 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

@openshift-ci

openshift-ci Bot commented Sep 15, 2026

Copy link
Copy Markdown

@leandroberetta: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/integration-tests cbf6f26 link true /test integration-tests

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

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