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

Upload qe images when code merged to main #4369

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

lilyLuLiu
Copy link
Contributor

@lilyLuLiu lilyLuLiu requested review from adrianriobo and removed request for praveenkumar and evidolob September 24, 2024 09:30
Copy link
Contributor

@adrianriobo adrianriobo left a comment

Choose a reason for hiding this comment

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

Can we extend https://github.com/crc-org/crc/blob/main/.github/workflows/windows-artifacts.yml#L58 as that is executed on push to main as well.

So we do not build images several times on push.

In this case you just add an extra step with condition on push event and re-tag images to next-OS-ARCH or dev-OS-ARCH just take into account:

https://gitlab.cee.redhat.com/crc/qe-platform/-/blob/master/catalog/task/crc-e2e/crc-e2e.yaml?ref_type=heads#L97

the task is composing the version with v as so it would expect i.e. vnext-OS-ARCH

As we also need to plan move crc-e2e task to ci-defintions we can re think on that v composition...meaning you can leave the tag as i.e. next-OS-ARCH, then we would need to adapt the version value accordingly.

WDYT?

@lilyLuLiu
Copy link
Contributor Author

lilyLuLiu commented Sep 25, 2024

@adrianriobo Hi, For the windows artifacts action(https://github.com/crc-org/crc/blob/main/.github/workflows/windows-artifacts.yml), I don't get your suggestion:
In the windows action, the qe images are built and upload to GitHub action artifacts, instead of quay. And the triggered windows test actions use the qe images from the uploaded artifacts, which requires the workflow id. So the two actions are different. Or you mean we can use quay qe images for windows github action tests?

@lilyLuLiu
Copy link
Contributor Author

@adrianriobo I have added V in the tag, like vNext-OS-ARCH. We can remove the v after migration to ci-definition.

@lilyLuLiu lilyLuLiu force-pushed the test_image_action branch 2 times, most recently from 4737620 to d4d7c16 Compare September 26, 2024 02:55
@lilyLuLiu lilyLuLiu changed the title add github action that upload qe images when code merged in test folder Upload qe images when code merged to main Sep 26, 2024
@lilyLuLiu lilyLuLiu force-pushed the test_image_action branch 2 times, most recently from f7f15e0 to fbd7378 Compare September 26, 2024 05:57
Copy link
Contributor

@adrianriobo adrianriobo left a comment

Choose a reason for hiding this comment

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

Can you change a bit the commit message, something like:

"[QE] Push qe oci image to quay on every merge to main.

This commit will push qe oci images to quay tagged as next, so we can use the latest changes on e2e and integration code to be used across all our CI systems"

In any case LGTM to me

Copy link

openshift-ci bot commented Sep 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adrianriobo

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

The pull request process is described 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

This commit will push qe oci images to quay tagged as next, so we can use the latest changes on e2e and integration code to be used across all our CI systems
Copy link

openshift-ci bot commented Sep 26, 2024

New changes are detected. LGTM label has been removed.

@adrianriobo adrianriobo merged commit ddb0e7c into crc-org:main Sep 26, 2024
21 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants