Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(e2): rhidp-6404 Add image check and relevant changes action to workflows #2575

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

gustavolira
Copy link
Contributor

@gustavolira gustavolira commented Mar 13, 2025

https://issues.redhat.com/browse/RHIDP-6404

Description

Please explain the changes you made here.

Which issue(s) does this PR fix

  • Fixes #?

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

Copy link

openshift-ci bot commented Mar 13, 2025

[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 fortune-ndlovu for approval. For more information see the Code Review Process.

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

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

Copy link
Contributor

Copy link
Contributor

@gustavolira gustavolira force-pushed the skip-build-packages branch from 6041833 to 7dcc740 Compare March 14, 2025 16:33
Copy link
Contributor

@gustavolira gustavolira force-pushed the skip-build-packages branch 2 times, most recently from 928ede2 to 2a05276 Compare March 17, 2025 14:01
Copy link
Contributor

Copy link
Contributor

@nickboldt nickboldt self-requested a review March 18, 2025 16:57
Copy link
Member

@nickboldt nickboldt left a comment

Choose a reason for hiding this comment

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

Only thing I don't like is that we're now doing the same GH checkout twice per action (first to check if we have to build, then to do the build)

🤔

if we moved the call to .github/actions/check-image-and-changes out of the check-image job and into the build-image job, then you might be able to reuse the same checkout for the build, and the step Check if build should be skipped would look at variable build-image.check-image.outputs.relevant_changes instead of needs.check-image.outputs.relevant_changes ?

@nickboldt
Copy link
Member

Might have a typo somewhere...

image

https://github.com/redhat-developer/rhdh/actions/runs/13928032057/job/38977666439

why was the job skipped? why is it incorrectly formatted?

@gustavolira gustavolira force-pushed the skip-build-packages branch from 16fc096 to 4bc297a Compare March 18, 2025 18:01
Copy link
Contributor

@gustavolira gustavolira requested a review from nickboldt March 18, 2025 21:53
Introduce a reusable `check-image-and-changes` composite action to determine if an image already exists and prioritize relevant changes for builds. Update workflows to integrate this action, conditionally triggering builds based on its outputs to optimize build processes.

Signed-off-by: Gustavo Lira <[email protected]>
Reorganized the workflow to include a dedicated job for checking image existence and relevant changes before building the image. Improved the logic for skipping builds when conditions are met and updated outputs to streamline data flow across steps. Removed repetitive actions and enhanced modularity for better maintainability.

Signed-off-by: Gustavo Lira <[email protected]>
Modified workflow logic to always execute build and test jobs. Added checks to conditionally skip execution when no relevant changes are detected.

Signed-off-by: Gustavo Lira <[email protected]>
Modified workflow logic to always execute build and test jobs. Added checks to conditionally skip execution when no relevant changes are detected.

Signed-off-by: Gustavo Lira <[email protected]>
Modified workflow logic to always execute build and test jobs. Added checks to conditionally skip execution when no relevant changes are detected.

Signed-off-by: Gustavo Lira <[email protected]>
@gustavolira gustavolira force-pushed the skip-build-packages branch from 4bc297a to 5c7596d Compare March 19, 2025 12:23
@gustavolira
Copy link
Contributor Author

Only thing I don't like is that we're now doing the same GH checkout twice per action (first to check if we have to build, then to do the build)

🤔

if we moved the call to .github/actions/check-image-and-changes out of the check-image job and into the build-image job, then you might be able to reuse the same checkout for the build, and the step Check if build should be skipped would look at variable build-image.check-image.outputs.relevant_changes instead of needs.check-image.outputs.relevant_changes ?

Thanks for the suggestion! We’ve implemented the improvement by moving the .github/actions/check-image-and-changes call directly into the build-image job. This allows us to reuse the same checkout step, avoiding duplicate operations. Now, the Check if build should be skipped step references build-image.check-image.outputs.relevant_changes instead of needs.check-image.outputs.relevant_changes

@gustavolira
Copy link
Contributor Author

Might have a typo somewhere...

image

https://github.com/redhat-developer/rhdh/actions/runs/13928032057/job/38977666439

why was the job skipped? why is it incorrectly formatted?

Fixed

Copy link
Contributor

id: check-image
uses: ./.github/actions/check-image-and-changes

- name: Check if build should be skipped
Copy link
Member

@nickboldt nickboldt Mar 20, 2025

Choose a reason for hiding this comment

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

a further simplication could be to move this check into the action itself.

then the output of the check-image-and-changes is not just two booleans, but an exit code of 0 if the job can be terminated right there.

Of course your logged message would need to be something generic about the job (rather than build or test) like

            echo "::notice::Skipping job: Image already exists and no relevant changes detected."

@nickboldt
Copy link
Member

Did a quick google for how to exit a GH workflow if something fails and it seems we won't get the results we want with an exit 0:

https://stackoverflow.com/questions/60589373/how-to-force-job-to-exit-in-github-actions-step

Instead of exiting 0, we might have to label all the subsequent steps with a conditional check to see if they should run...

steps.check.conclusion == 'failure'

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.

2 participants